[PATCH 1/2] Add a check for unnecessary markup.

David Shea dshea at redhat.com
Tue Mar 4 20:24:03 UTC 2014


If markup is present that could be expressed as Pango attributes, fail.
This gets markup out of translatable strings and makes it impossible to
mis-translate.

The check for unnecessary markup in Python is commented out, because
it's impossible to use Pango from Python in gobject-introspection. See
https://bugzilla.gnome.org/show_bug.cgi?id=725681.
---
 tests/glade/markup/check_markup.py |  8 ++++-
 tests/lib/pangocheck.py            | 60 ++++++++++++++++++++++++++++++++++++++
 tests/pylint/markup.py             | 16 ++++++++--
 3 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/tests/glade/markup/check_markup.py b/tests/glade/markup/check_markup.py
index 3406715..c90d08d 100755
--- a/tests/glade/markup/check_markup.py
+++ b/tests/glade/markup/check_markup.py
@@ -34,7 +34,7 @@ if ('-t' in sys.argv) or ('--translate' in sys.argv):
         print("Unable to load po translation module")
         sys.exit(99)
 
-from pangocheck import markup_nodes, markup_match
+from pangocheck import markup_nodes, markup_match, markup_necessary
 
 try:
     from lxml import etree
@@ -86,6 +86,12 @@ def check_glade_file(glade_file_path, po_map=None):
                     # pylint: disable=W9922
                     pango_tree = etree.fromstring("<markup>%s</markup>" % label_text)
                     _validate_pango_markup(pango_tree)
+
+                    # Check if the markup is necessary
+                    if not markup_necessary(pango_tree):
+                        print("Markup could be expressed as attributes at %s%s:%d" % \
+                                (glade_file_path, lang_str, label.sourceline))
+                        glade_success = False
                 except etree.XMLSyntaxError:
                     print("Unable to parse pango markup at %s%s:%d" % \
                             (glade_file_path, lang_str, label.sourceline))
diff --git a/tests/lib/pangocheck.py b/tests/lib/pangocheck.py
index 368d1db..75aa307 100644
--- a/tests/lib/pangocheck.py
+++ b/tests/lib/pangocheck.py
@@ -64,3 +64,63 @@ def markup_match(orig_markup, xlated_markup):
     attr_list2 = sorted(attr_count2.elements())
 
     return (name_list1 == name_list2) and (attr_list1 == attr_list2)
+
+# Check that the markup is needed at all.
+# The input is a parsed ElementTree of the string '<markup>pango markup goes here</markup>'
+# The markup is unnecessary if the only markup in the string surrounds the entire rest of
+# the string, meaning that the pango attributes apply to the entire string, and thus
+# could be expressed using attribute lists. For example, strings like:
+#   <b>Bold text</b>
+# or
+#   <span foreground="grey"><i>colorful</i></span>
+# but not strings like:
+#   <span size="small">This string contains <b>internal</b> markup</span>
+# that contain markup that must be passed to the translators.
+#
+# This function returns True if the markup is necessary and False if the markup
+# can be discarded and expressed as attribute lists.
+def markup_necessary(markup_tree):
+    # If the element has no children at all, there is no markup inside and the
+    # markup is unnecessary.
+    if not len(markup_tree):
+        return False
+
+    # If there is more than one child, the markup is necessary
+    if len(markup_tree) > 1:
+        return True
+
+    # QUICK NOTE FOR PEOPLE EXPECTING ElementTree TO ACT KINDA LIKE DOM 'CUZ LOL
+    # ElementTree is kind of weird with respect to handling multiple text children
+    # of an Element node. element.text is the text leading up to the first element
+    # child, and element[child_idx].tail is the text in element following the
+    # child node. So child.tail is actually a child of element but it isn't a
+    # property of element because Python is crazy.
+    #
+    # A string like "<markup>word1<i>word2</i>word3<empty/>word4</markup>" will result in
+    #   tree == <Element 'markup' ...>
+    #   tree.text == 'word1'
+    #   tree[0] == <Element 'i' ...>
+    #   tree[0].text == 'word2'
+    #   tree[0].tail == 'word3'
+    #   tree[1] == <Element 'empty' ...>
+    #   tree[1].text == None
+    #   tree[1].text == 'word4'
+    #
+    # So elements that contain text before a child markup element will have
+    # element.text is not None. Elements that have text after a child element
+    # will have .tail on that child set to not None.
+
+    # If .text is set, there is text before the child node, as in
+    # <span>text <b>child</b></span>
+    # and the markup is necessary
+    if markup_tree.text:
+        return True
+
+    # If the child (we already know there's only one) has .tail set, then
+    # there is text between the close of the child and the end of the element
+    # and the markup is necessary
+    if markup_tree[0].tail:
+        return True
+
+    # Recurse on the child node
+    return markup_necessary(markup_tree[0])
diff --git a/tests/pylint/markup.py b/tests/pylint/markup.py
index c0e4489..82ed332 100644
--- a/tests/pylint/markup.py
+++ b/tests/pylint/markup.py
@@ -31,7 +31,8 @@ import os
 
 import xml.etree.ElementTree as ET
 
-from pangocheck import markup_nodes, is_markup, markup_match
+# markup_necessary not used yet
+from pangocheck import markup_nodes, is_markup, markup_match #, markup_necessary
 
 markupMethods = ["set_markup"]
 escapeMethods = ["escape_markup"]
@@ -62,7 +63,10 @@ class MarkupChecker(BaseChecker):
                        "Translated pango markup contains invalid elements"),
             "W9925" : ("Found mis-translated pango markup for language %s",
                        "invalid-pango-translation",
-                       "The elements or attributes do not match between a pango markup string and its translation")
+                       "The elements or attributes do not match between a pango markup string and its translation"),
+            "W9926" : ("Found unnecessary pango markup",
+                       "unnecessary-markup",
+                       "Pango markup could be expressed as attribute list"),
            }
 
     options = (('translate-markup',
@@ -88,6 +92,12 @@ class MarkupChecker(BaseChecker):
             # QUIS CUSTODIET IPSOS CUSTODES
             # pylint: disable=W9922
             tree = ET.fromstring("<markup>%s</markup>" % string)
+
+            # Check if the markup is necessary
+            # TODO: Turn this on after it's possible to actually do
+            # anything about it. See https://bugzilla.gnome.org/show_bug.cgi?id=725681
+            #if not markup_necessary(tree):
+            #    self.add_message("W9926", node=node)
         except ET.ParseError:
             if lang:
                 self.add_message("W9923", node=node, args=(lang,))
@@ -100,7 +110,7 @@ class MarkupChecker(BaseChecker):
     def __init__(self, linter=None):
         BaseChecker.__init__(self, linter)
 
-    @check_messages("W9920", "W9921", "W9922", "W9923", "W9924", "W9925")
+    @check_messages("W9920", "W9921", "W9922", "W9923", "W9924", "W9925", "W9926")
     def visit_const(self, node):
         if type(node.value) not in (types.StringType, types.UnicodeType):
             return
-- 
1.8.5.3



More information about the anaconda-patches mailing list