This removes the need for most of the exception comments. The only exceptions left are one in installation source where elements are shown and hidden based on a radio button selection, and one in the iscsi login where elements are shown and hidden based on a dropdown. --- pyanaconda/ui/gui/spokes/advstorage/iscsi.glade | 11 +- pyanaconda/ui/gui/spokes/custom.glade | 2 - pyanaconda/ui/gui/spokes/filter.glade | 7 -- pyanaconda/ui/gui/spokes/network.glade | 4 - tests/accelerators/check_accelerators.py | 150 ++++++++++++++++++------ 5 files changed, 117 insertions(+), 57 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/advstorage/iscsi.glade b/pyanaconda/ui/gui/spokes/advstorage/iscsi.glade index 98fb46f..208122a 100644 --- a/pyanaconda/ui/gui/spokes/advstorage/iscsi.glade +++ b/pyanaconda/ui/gui/spokes/advstorage/iscsi.glade @@ -397,7 +397,6 @@ <property name="visible">True</property> <property name="can_focus">False</property> <property name="xalign">0</property> - <!-- check_accelerators: not visible at same time as label2 --> <property name="label" translatable="yes">CHAP _Username:</property> <property name="use_underline">True</property> <property name="mnemonic_widget">rchapUsernameEntry</property> @@ -414,7 +413,6 @@ <property name="visible">True</property> <property name="can_focus">False</property> <property name="xalign">0</property> - <!-- check_accelerators: not visible at same time as label3 --> <property name="label" translatable="yes">CHAP _Password:</property> <property name="use_underline">True</property> <property name="mnemonic_widget">rchapPasswordEntry</property> @@ -764,7 +762,6 @@ <property name="visible">True</property> <property name="can_focus">False</property> <property name="xalign">0</property> - <!-- check_accelerators: not visible at same time as label11 --> <property name="label" translatable="yes">_Node login authentication type:</property> <property name="use_underline">True</property> <property name="mnemonic_widget">loginAuthTypeCombo</property> @@ -823,7 +820,6 @@ <property name="visible">True</property> <property name="can_focus">False</property> <property name="xalign">0</property> - <!-- check_accelerators: not visible at same time as label2 --> <property name="label" translatable="yes">CHAP _Username:</property> <property name="use_underline">True</property> <property name="mnemonic_widget">loginChapUsernameEntry</property> @@ -840,7 +836,6 @@ <property name="visible">True</property> <property name="can_focus">False</property> <property name="xalign">0</property> - <!-- check_accelerators: not visible at same time as label3 --> <property name="label" translatable="yes">CHAP _Password:</property> <property name="use_underline">True</property> <property name="mnemonic_widget">loginChapPasswordEntry</property> @@ -967,7 +962,6 @@ <property name="visible">True</property> <property name="can_focus">False</property> <property name="xalign">0</property> - <!-- check_accelerators: not visible at same time as label2 --> <property name="label" translatable="yes">CHAP _Username:</property> <property name="use_underline">True</property> <property name="mnemonic_widget">loginRchapUsernameEntry</property> @@ -984,7 +978,6 @@ <property name="visible">True</property> <property name="can_focus">False</property> <property name="xalign">0</property> - <!-- check_accelerators: not visible at same time as label3 --> <property name="label" translatable="yes">CHAP _Password:</property> <property name="use_underline">True</property> <property name="mnemonic_widget">loginRchapPasswordEntry</property> @@ -1001,7 +994,6 @@ <property name="visible">True</property> <property name="can_focus">False</property> <property name="xalign">0</property> - <!-- check_accelerators: not visible at same time as retryButton --> <property name="label" translatable="yes">_Reverse CHAP Username:</property> <property name="use_underline">True</property> <property name="mnemonic_widget">loginRchapReverseUsername</property> @@ -1018,7 +1010,6 @@ <property name="visible">True</property> <property name="can_focus">False</property> <property name="xalign">0</property> - <!-- check_accelerators: not visible at same time as label12 --> <property name="label" translatable="yes">Reverse CHAP Pass_word:</property> <property name="use_underline">True</property> <property name="mnemonic_widget">loginRchapReversePassword</property> @@ -1170,7 +1161,7 @@ </child> <child> <object class="GtkButton" id="retryLoginButton"> - <!-- check_accelerators: not visible at same time as retryButton --> + <!-- check_accelerators: not visible at same time as label19 --> <property name="label" translatable="yes">_Retry Log In</property> <property name="visible">True</property> <property name="can_focus">True</property> diff --git a/pyanaconda/ui/gui/spokes/custom.glade b/pyanaconda/ui/gui/spokes/custom.glade index 1b5de13..2c45c5f 100644 --- a/pyanaconda/ui/gui/spokes/custom.glade +++ b/pyanaconda/ui/gui/spokes/custom.glade @@ -1364,7 +1364,6 @@ until you click on the main menu's 'Begin Installation' button.</property> <object class="GtkLabel" id="passphraseLabel"> <property name="visible">True</property> <property name="can_focus">False</property> - <!-- check_accelerators: not visible at same time as mountPointLabel --> <property name="label" translatable="yes">_Passphrase:</property> <property name="use_underline">True</property> <attributes> @@ -1393,7 +1392,6 @@ until you click on the main menu's 'Begin Installation' button.</property> </child> <child> <object class="GtkButton" id="unlockButton"> - <!-- check_accelerators: not visible at same time as applyButton --> <property name="label" translatable="yes">_Unlock</property> <property name="visible">True</property> <property name="can_focus">True</property> diff --git a/pyanaconda/ui/gui/spokes/filter.glade b/pyanaconda/ui/gui/spokes/filter.glade index e98ed1a..8c50349 100644 --- a/pyanaconda/ui/gui/spokes/filter.glade +++ b/pyanaconda/ui/gui/spokes/filter.glade @@ -641,7 +641,6 @@ <object class="GtkLabel" id="label16"> <property name="visible">True</property> <property name="can_focus">False</property> - <!-- check_accelerators: not visible at same time as label7 --> <property name="label" translatable="yes">Show Only _Devices With:</property> <property name="use_underline">True</property> <property name="mnemonic_widget">multipathInterconnectCombo</property> @@ -682,7 +681,6 @@ <object class="GtkLabel" id="label17"> <property name="visible">True</property> <property name="can_focus">False</property> - <!-- check_accelerators: not visible at same time as label7 --> <property name="label" translatable="yes">Show Only _Devices Containing:</property> <property name="use_underline">True</property> <property name="mnemonic_widget">multipathWWIDEntry</property> @@ -901,7 +899,6 @@ <property name="visible">True</property> <property name="can_focus">False</property> <property name="xalign">0</property> - <!-- check_accelerators: not visible at same time as label2 --> <property name="label" translatable="yes">_Filter By:</property> <property name="use_underline">True</property> <property name="mnemonic_widget">multipathTypeCombo</property> @@ -960,7 +957,6 @@ <object class="GtkLabel" id="label12"> <property name="visible">True</property> <property name="can_focus">False</property> - <!-- check_accelerators: not visible at same time as label7 --> <property name="label" translatable="yes">Show Only _Devices From:</property> <property name="use_underline">True</property> <property name="mnemonic_widget">otherVendorCombo</property> @@ -1001,7 +997,6 @@ <object class="GtkLabel" id="label20"> <property name="visible">True</property> <property name="can_focus">False</property> - <!-- check_accelerators: not visible at same time as label7 --> <property name="label" translatable="yes">Show Only _Devices With:</property> <property name="use_underline">True</property> <property name="mnemonic_widget">otherInterconnectCombo</property> @@ -1042,7 +1037,6 @@ <object class="GtkLabel" id="label21"> <property name="visible">True</property> <property name="can_focus">False</property> - <!-- check_accelerators: not visible at same time as label7 --> <property name="label" translatable="yes">Show Only _Devices Containing:</property> <property name="use_underline">True</property> <property name="mnemonic_widget">otherIDEntry</property> @@ -1272,7 +1266,6 @@ <property name="visible">True</property> <property name="can_focus">False</property> <property name="xalign">0</property> - <!-- check_accelerators: not visible at same time as label2 --> <property name="label" translatable="yes">_Filter By:</property> <property name="use_underline">True</property> <property name="mnemonic_widget">zFilterCombo</property> diff --git a/pyanaconda/ui/gui/spokes/network.glade b/pyanaconda/ui/gui/spokes/network.glade index 14ec157..5701e68 100644 --- a/pyanaconda/ui/gui/spokes/network.glade +++ b/pyanaconda/ui/gui/spokes/network.glade @@ -1641,7 +1641,6 @@ updates available for you.</property> <property name="top_padding">12</property> <child> <object class="GtkButton" id="button_mobilebb_options"> - <!-- check_accelerators: not in same tab as button_wireless_options --> <property name="label" translatable="yes">_Configure...</property> <property name="visible">True</property> <property name="can_focus">True</property> @@ -1927,7 +1926,6 @@ updates available for you.</property> <property name="top_padding">12</property> <child> <object class="GtkButton" id="button_vpn_options"> - <!-- check_accelerators: not in same tab as button_wireless_options --> <property name="label" translatable="yes">_Configure...</property> <property name="visible">True</property> <property name="can_focus">True</property> @@ -2083,7 +2081,6 @@ updates available for you.</property> <property name="visible">True</property> <property name="can_focus">False</property> <property name="xalign">1</property> - <!-- check_accelerators: not in same tab as button_wireless_options --> <property name="label" translatable="yes">_Configuration URL</property> <property name="use_underline">True</property> <property name="mnemonic_widget">entry_proxy_url</property> @@ -2162,7 +2159,6 @@ updates available for you.</property> <property name="visible">True</property> <property name="can_focus">False</property> <property name="xalign">1</property> - <!-- check_accelerators: not in same tab as button_wireless_options --> <property name="label" translatable="yes">_Socks Host</property> <property name="use_underline">True</property> <property name="mnemonic_widget">entry_proxy_socks</property> diff --git a/tests/accelerators/check_accelerators.py b/tests/accelerators/check_accelerators.py index dff3c3f..e4a5f56 100755 --- a/tests/accelerators/check_accelerators.py +++ b/tests/accelerators/check_accelerators.py @@ -21,6 +21,7 @@ import sys import argparse import re import os.path +import copy
try: from lxml import etree @@ -30,55 +31,136 @@ except ImportError:
accel_re = re.compile(r'_(?P<accel>.)')
+success = True +def add_check_accel(glade_filename, accels, label): + """Check whether an accelerator conflicts with existing accelerators. + and add it to the current accelerator context. + """ + global success + + match = accel_re.search(label.text) + if match: + accel = match.group('accel').lower() + if accel in accels: + # Check for an exception comment + prev = label.getprevious() + if (prev is not None) and (prev.tag == etree.Comment) and \ + prev.text.strip().startswith('check_accelerators:'): + return + + print("Accelerator collision for key %s in %s\n line %d: %s\n line %d: %s" %\ + (accel, os.path.normpath(glade_filename), + accels[accel].sourceline, accels[accel].text, + label.sourceline, label.text)) + success = False + else: + accels[accel] = label + +def combine_accels(glade_filename, list_a, list_b): + if not list_a: + return list_b + if not list_b: + return list_a + + newlist = [] + for accels_a in list_a: + for accels_b in list_b: + new_accels = copy.copy(accels_a) + for accel in accels_b.keys(): + add_check_accel(glade_filename, new_accels, accels_b[accel]) + newlist.append(new_accels) + return newlist + +# GtkNotebook widgets define several child widgets, not all of which are active +# at the same time. To further complicate things, an object can have more than +# one GtkNotebook child, and a GtkNotebook and have GtkNotebook children. +# +# To handle this, GtkNotebook objects are processed separately. +# process_notebook returns a list of possible accelerator lists, and each of +# these is compared against the list of accelerators returned for the object's +# other GtkNotebook children. + +def process_object(glade_filename, interface_object): + """Process keyboard shortcuts for a given glade object. + + The return value from this function is a list of accelerator + dictionaries, with each consiting of accelerator shortcut characters + as keys and the corresponding <object> Element objects as values. Each + dictionary represents a set of accelerators that could be active at any + given time. + """ + # Start with an empty context for things that are always active + accels = [{}] + + # Add everything that isn't a child of a GtkNotebook + for label in interface_object.xpath(".//property[@name='label' and ../property[@name='use_underline']/text() = 'True' and not(ancestor::object[@class='GtkNotebook'])]"): + add_check_accel(glade_filename, accels[0], label) + + # For each GtkNotebook tab that is not a child of another notebook, + # add the tab to the top-level context + for notebook_label in interface_object.xpath(".//object[@class='GtkNotebook' and not(ancestor::object[@class='GtkNotebook'])]/child[@type='tab']//property[@name='label' and ../property[@name='use_underline']/text() = 'True']"): + add_check_accel(glade_filename, accels[0], notebook_label) + + # Now process each non-tab object in each Gtknotebook that is not a child + # of another notebook. For each Gtk notebook, each non-tab child represents + # a separate potentially-active context. Since each list returned by + # process_object for a GtkNotebook child is independent of each other + # GtkNotebook child, we can just combine all of them into a single list. + # For example, say there's a notebook with two panes. The first pane + # contains another notebook with two panes. Let's call the main pane + # A, and the panes in the child GtkNotebook A_1 and A_2. A contains an + # accelerator for 'a', A_1 contains accelerators for 'b' and 'c', and A_2 + # contains accelerators for 'b' and 'c'. The list returned would look like: + # [{'a': label1, 'b': label2, 'c': label3}, + # {'a': label1, 'b': label4, 'c': label5}] + # Then when we process the other pane in the Notebook that contains A + # (let's call it B), we find acclerators for 'a' and 'b': + # [{'a': label6, 'b': label7}]. + # None of these dictionaries are active at the same time. Because + # process_object on A combined the accelerators that are in the top-level + # of A with each of the accelerators in the Notebook children of A, we can + # treat A as if it were actually two panes at the same-level of B and just + # create a list of three dictionaries for the whole notebook. + # + # A deepcopy of each object is taken so that the object can be treated as a + # separate XML document so that the ancestor axis stuff works. + + for notebook in interface_object.xpath(".//object[@class='GtkNotebook' and not(ancestor::object[@class='GtkNotebook'])]"): + # Create the list of dictionaries for the notebook + notebook_list = [] + for child in notebook.xpath("./child[not(@type='tab')]"): + notebook_list.extend(process_object(glade_filename, copy.deepcopy(child))) + + # Now combine this with our list of accelerators + accels = combine_accels(glade_filename, accels, notebook_list) + + return accels + def check_glade(glade_filename): - success = True with open(glade_filename) as glade_file: # Parse the XML glade_tree = etree.parse(glade_file)
- # Assume that each top-level object is a separate context + # Treat each top-level object as a separate context for interface_object in glade_tree.xpath("/interface/object"): - accels = {} - - # Select the label properties of objects whose use_underline - # property is true. - # NB: This is why we can't use python's builtin etree - for label in interface_object.xpath(".//property[@name='label' and ../property[@name='use_underline']/text() = 'True']"): - match = accel_re.search(label.text) - if match: - accel = match.group('accel').lower() - if accel in accels: - # Check for an exception comment - prev = label.getprevious() - if (prev is not None) and (prev.tag == etree.Comment) and \ - prev.text.strip().startswith('check_accelerators:'): - continue - - print("Accelerator collision for key %s in %s:%s\n line %d: %s\n line %d: %s" %\ - (accel, os.path.normpath(glade_filename), interface_object.attrib['id'], - accels[accel].sourceline, accels[accel].text, - label.sourceline, label.text)) - success = False - else: - accels[accel] = label - return success - -def main(argv=sys.argv): + process_object(glade_filename, interface_object) + +def main(argv=None): + if argv is None: + argv = sys.argv + parser = argparse.ArgumentParser("Check for duplicated accelerators") parser.add_argument("glade_files", nargs="+", metavar="GLADE-FILE", help='The glade file to check') args = parser.parse_args(args=argv)
- success = True for glade_file in args.glade_files: - if not check_glade(glade_file): - success = False - return success + check_glade(glade_file)
if __name__ == "__main__": - result = main(sys.argv[1:]) + main(sys.argv[1:])
- if result: + if success: sys.exit(0) else: sys.exit(1)
On Fri, Sep 20, 2013 at 03:50:00PM -0400, David Shea wrote:
This removes the need for most of the exception comments. The only exceptions left are one in installation source where elements are shown and hidden based on a radio button selection, and one in the iscsi login where elements are shown and hidden based on a dropdown.
pyanaconda/ui/gui/spokes/advstorage/iscsi.glade | 11 +- pyanaconda/ui/gui/spokes/custom.glade | 2 - pyanaconda/ui/gui/spokes/filter.glade | 7 -- pyanaconda/ui/gui/spokes/network.glade | 4 - tests/accelerators/check_accelerators.py | 150 ++++++++++++++++++------ 5 files changed, 117 insertions(+), 57 deletions(-)
Ack
anaconda-patches@lists.fedorahosted.org
-
Brian C. Lane -
David Shea