directly extract macros to lists

so far imported macro element were inserted into the macros element
of the tool xml .. just to be removed again in the end (except for
template macros)

so it seems cleaner (and maybe a bit faster) to store the macros
in lists (kept in a dict separating xml, token, and template macros)

lib/galaxy/util/xml_macros.py

@@ -19,28 +19,25 @@ def load_with_references(path):
     if macros_el is None:
         return tree, []
 
-    # move the macros element to a separate tree
-    # otherwise included expand elements would be treated
-    # in the macro expansion
-    macros_tree = etree.ElementTree(copy(macros_el))
+    macros = {}
+    macro_paths = _import_macros(macros_el, path, macros)
     macros_el.clear()
-    org_macros_el = macros_el
-    macros_el = macros_tree.getroot()
-
-    macro_paths = _import_macros(macros_el, path)
 
     # Collect tokens
-    tokens = _macros_of_type(macros_el, 'token', lambda el: el.text or '')
+    tokens = {}
+    for m in macros.get('token', []):
+        tokens[m.get("name")] = m.text or ""
     tokens = expand_nested_tokens(tokens)
 
     # Expand xml macros
-    macro_dict = _macros_of_type(macros_el, 'xml', lambda el: XmlMacroDef(el))
+    macro_dict = {}
+    for m in macros.get('xml', []):
+        macro_dict[m.get("name")] = XmlMacroDef(m)
     _expand_macros([root], macro_dict, tokens)
 
     # reinsert template macro which are used during tool execution
-    for el in macros_el.xpath('//macro'):
-        if el.get('type') == 'template':
-            org_macros_el.append(el)
+    for m in macros.get("template", []):
+        macros_el.append(m)
     _expand_tokens_for_el(root, tokens)
     return tree, macro_paths
 
@@ -76,15 +73,15 @@ def imported_macro_paths(root):
     return _imported_macro_paths_from_el(macros_el)
 
 
-def _import_macros(macros_el, path):
+def _import_macros(macros_el, path, macros):
     """
     root the parsed XML tree
     path the path to the main xml document
     """
     xml_base_dir = os.path.dirname(path)
     if macros_el is not None:
-        macro_els, macro_paths = _load_macros(macros_el, xml_base_dir)
-        _xml_set_children(macros_el, macro_els)
+        macro_paths = _load_macros(macros_el, xml_base_dir, macros)
+        # _xml_set_children(macros_el, macro_els)
         return macro_paths
 
 
@@ -214,55 +211,48 @@ def _expand_yield_statements(macro_def, expand_el):
         _xml_replace(yield_el, expand_el_children)
 
 
-def _load_macros(macros_el, xml_base_dir):
-    macros = []
+def _load_macros(macros_el, xml_base_dir, macros):
     # Import macros from external files.
-    imported_macros, macro_paths = _load_imported_macros(macros_el, xml_base_dir)
-    macros.extend(imported_macros)
+    macro_paths = _load_imported_macros(macros_el, xml_base_dir, macros)
     # Load all directly defined macros.
-    macros.extend(_load_embedded_macros(macros_el, xml_base_dir))
-    return macros, macro_paths
-
+    _load_embedded_macros(macros_el, xml_base_dir, macros)
+    return macro_paths
 
-def _load_embedded_macros(macros_el, xml_base_dir):
-    macros = []
 
-    macro_els = []
+def _load_embedded_macros(macros_el, xml_base_dir, macros):
+    if macros_el is None:
+        return
     # attribute typed macro
-    if macros_el is not None:
-        macro_els = macros_el.findall("macro")
-    for macro in macro_els:
+    for macro in macros_el.iterfind("macro"):
         if 'type' not in macro.attrib:
             macro.attrib['type'] = 'xml'
-        macros.append(macro)
+        try:
+            macros[macro.attrib['type']].append(macro)
+        except KeyError:
+            macros[macro.attrib['type']] = [macro]
 
     # type shortcuts (<xml> is a shortcut for <macro type="xml",
     # likewise for <template>.
-    typed_tag = ['template', 'xml', 'token']
-    for tag in typed_tag:
-        macro_els = []
-        if macros_el is not None:
-            macro_els = macros_el.findall(tag)
-        for macro_el in macro_els:
+    for tag in ['template', 'xml', 'token']:
+        for macro_el in macros_el.iterfind(tag):
             macro_el.attrib['type'] = tag
             macro_el.tag = 'macro'
-            macros.append(macro_el)
-
-    return macros
+            try:
+                macros[tag].append(macro_el)
+            except KeyError:
+                macros[tag] = [macro_el]
 
 
-def _load_imported_macros(macros_el, xml_base_dir):
-    macros = []
+def _load_imported_macros(macros_el, xml_base_dir, macros):
     macro_paths = []
 
     for tool_relative_import_path in _imported_macro_paths_from_el(macros_el):
         import_path = \
             os.path.join(xml_base_dir, tool_relative_import_path)
         macro_paths.append(import_path)
-        file_macros, current_macro_paths = _load_macro_file(import_path, xml_base_dir)
-        macros.extend(file_macros)
+        current_macro_paths = _load_macro_file(import_path, xml_base_dir, macros)
         macro_paths.extend(current_macro_paths)
-    return macros, macro_paths
+    return macro_paths
 
 
 def _imported_macro_paths_from_el(macros_el):
@@ -276,10 +266,10 @@ def _imported_macro_paths_from_el(macros_el):
     return imported_macro_paths
 
 
-def _load_macro_file(path, xml_base_dir):
+def _load_macro_file(path, xml_base_dir, macros):
     tree = parse_xml(path, strip_whitespace=False)
     root = tree.getroot()
-    return _load_macros(root, xml_base_dir)
+    return _load_macros(root, xml_base_dir, macros)
 
 
 def _xml_set_children(element, new_children):