refactor add_commands() out into commands module

9 files changed, 282 insertions, 263 deletions

diff --git a/lib/commands.py b/lib/commands.py
index 3c739708..ddee8326 100644
--- a/lib/commands.py
+++ b/lib/commands.py
@@ -1,12 +1,18 @@
+from copy import deepcopy
+import os
+from random import random
 import sys
-import inkex
+
 import cubicsuperpath
+import inkex
 import simpletransform
 
-from .svg import apply_transforms, get_node_transform
-from .svg.tags import SVG_USE_TAG, SVG_SYMBOL_TAG, CONNECTION_START, CONNECTION_END, XLINK_HREF
-from .utils import cache, Point
 from .i18n import _, N_
+from .svg import apply_transforms, get_node_transform, get_correction_transform, get_document, generate_unique_id
+from .svg.tags import SVG_DEFS_TAG, SVG_GROUP_TAG, SVG_PATH_TAG, SVG_USE_TAG, SVG_SYMBOL_TAG, \
+    CONNECTION_START, CONNECTION_END, CONNECTOR_TYPE, XLINK_HREF, INKSCAPE_LABEL
+from .utils import cache, get_bundled_dir, Point
+
 
 COMMANDS = {
     # L10N command attached to an object
@@ -228,3 +234,125 @@ def _standalone_commands(svg):
 
 def is_command(node):
     return CONNECTION_START in node.attrib or CONNECTION_END in node.attrib
+
+
+@cache
+def symbols_path():
+    return os.path.join(get_bundled_dir("symbols"), "inkstitch.svg")
+
+
+@cache
+def symbols_svg():
+    with open(symbols_path()) as symbols_file:
+        return inkex.etree.parse(symbols_file)
+
+
+@cache
+def symbol_defs():
+    return get_defs(symbols_svg())
+
+
+@cache
+def get_defs(document):
+    return document.find(SVG_DEFS_TAG)
+
+
+def ensure_symbol(document, command):
+    """Make sure the command's symbol definition exists in the <svg:defs> tag."""
+
+    path = "./*[@id='inkstitch_%s']" % command
+    defs = get_defs(document)
+    if defs.find(path) is None:
+        defs.append(deepcopy(symbol_defs().find(path)))
+
+
+def add_group(document, node, command):
+    return inkex.etree.SubElement(
+        node.getparent(),
+        SVG_GROUP_TAG,
+        {
+            "id": generate_unique_id(document, "group"),
+            INKSCAPE_LABEL: _("Ink/Stitch Command") + ": %s" % get_command_description(command),
+            "transform": get_correction_transform(node)
+        })
+
+
+def add_connector(document, symbol, element):
+    # I'd like it if I could position the connector endpoint nicely but inkscape just
+    # moves it to the element's center immediately after the extension runs.
+    start_pos = (symbol.get('x'), symbol.get('y'))
+    end_pos = element.shape.centroid
+
+    path = inkex.etree.Element(SVG_PATH_TAG,
+                               {
+                                   "id": generate_unique_id(document, "connector"),
+                                   "d": "M %s,%s %s,%s" % (start_pos[0], start_pos[1], end_pos.x, end_pos.y),
+                                   "style": "stroke:#000000;stroke-width:1px;stroke-opacity:0.5;fill:none;",
+                                   CONNECTION_START: "#%s" % symbol.get('id'),
+                                   CONNECTION_END: "#%s" % element.node.get('id'),
+                                   CONNECTOR_TYPE: "polyline",
+
+                                   # l10n: the name of the line that connects a command to the object it applies to
+                                   INKSCAPE_LABEL: _("connector")
+                               })
+
+    symbol.getparent().insert(0, path)
+
+
+def add_symbol(document, group, command, pos):
+    return inkex.etree.SubElement(group, SVG_USE_TAG,
+                                  {
+                                      "id": generate_unique_id(document, "use"),
+                                      XLINK_HREF: "#inkstitch_%s" % command,
+                                      "height": "100%",
+                                      "width": "100%",
+                                      "x": str(pos.x),
+                                      "y": str(pos.y),
+
+                                      # l10n: the name of a command symbol (example: scissors icon for trim command)
+                                      INKSCAPE_LABEL: _("command marker"),
+                                  })
+
+
+def get_command_pos(element, index, total):
+    # Put command symbols 30 pixels out from the shape, spaced evenly around it.
+
+    # get a line running 30 pixels out from the shape
+    outline = element.shape.buffer(30).exterior
+
+    # pick this item's spot arond the outline and perturb it a bit to avoid
+    # stacking up commands if they run the extension multiple times
+    position = index / float(total)
+    position += random() * 0.1
+
+    return outline.interpolate(position, normalized=True)
+
+
+def remove_legacy_param(element, command):
+    if command == "trim" or command == "stop":
+        # If they had the old "TRIM after" or "STOP after" attributes set,
+        # automatically delete them.  THe new commands will do the same
+        # thing.
+        #
+        # If we didn't delete these here, then things would get confusing.
+        # If the user were to delete a "trim" symbol added by this extension
+        # but the "embroider_trim_after" attribute is still set, then the
+        # trim would keep happening.
+
+        attribute = "embroider_%s_after" % command
+
+        if attribute in element.node.attrib:
+            del element.node.attrib[attribute]
+
+
+def add_commands(element, commands):
+    document = get_document(element.node)
+
+    for i, command in enumerate(commands):
+        ensure_symbol(document, command)
+        remove_legacy_param(element, command)
+
+        group = add_group(document, element.node, command)
+        pos = get_command_pos(element, i, len(commands))
+        symbol = add_symbol(document, group, command, pos)
+        add_connector(document, symbol, element)
diff --git a/lib/extensions/auto_satin.py b/lib/extensions/auto_satin.py
index f846ac6b..90d8fe33 100644
--- a/lib/extensions/auto_satin.py
+++ b/lib/extensions/auto_satin.py
@@ -2,6 +2,7 @@ import sys
 
 import inkex
 
+from ..commands import add_commands
 from ..elements import SatinColumn
 from ..i18n import _
 from ..stitches.auto_satin import auto_satin
@@ -97,6 +98,5 @@ class AutoSatin(CommandsExtension):
 
     def add_trims(self, new_elements, trim_indices):
         if self.options.trim and trim_indices:
-            self.ensure_symbol("trim")
             for i in trim_indices:
-                self.add_commands(new_elements[i], ["trim"])
+                add_commands(new_elements[i], ["trim"])
diff --git a/lib/extensions/base.py b/lib/extensions/base.py
index 98673541..8d45f790 100644
--- a/lib/extensions/base.py
+++ b/lib/extensions/base.py
@@ -9,6 +9,7 @@ from stringcase import snakecase
 from ..commands import layer_commands
 from ..elements import EmbroideryElement, nodes_to_elements
 from ..i18n import _
+from ..svg import generate_unique_id
 from ..svg.tags import SVG_GROUP_TAG, INKSCAPE_GROUPMODE, SVG_DEFS_TAG, EMBROIDERABLE_TAGS
 
 
@@ -194,15 +195,7 @@ class InkstitchExtension(inkex.Effect):
 
     def uniqueId(self, prefix, make_new_id=True):
         """Override inkex.Effect.uniqueId with a nicer naming scheme."""
-        i = 1
-        while True:
-            new_id = "%s%d" % (prefix, i)
-            if new_id not in self.doc_ids:
-                break
-            i += 1
-        self.doc_ids[new_id] = 1
-
-        return new_id
+        return generate_unique_id(self.document, prefix)
 
     def parse(self):
         """Override inkex.Effect.parse to add Ink/Stitch xml namespace"""
diff --git a/lib/extensions/commands.py b/lib/extensions/commands.py
index 07b450e1..86e291fd 100644
--- a/lib/extensions/commands.py
+++ b/lib/extensions/commands.py
@@ -1,16 +1,4 @@
-import os
-import inkex
-from copy import deepcopy
-from random import random
-
-
 from .base import InkstitchExtension
-from ..utils import get_bundled_dir, cache
-from ..commands import get_command_description
-from ..i18n import _
-from ..svg.tags import SVG_DEFS_TAG, SVG_PATH_TAG, CONNECTION_START, CONNECTION_END, \
-                       CONNECTOR_TYPE, INKSCAPE_LABEL, SVG_GROUP_TAG, SVG_USE_TAG, XLINK_HREF
-from ..svg import get_correction_transform
 
 
 class CommandsExtension(InkstitchExtension):
@@ -20,109 +8,3 @@ class CommandsExtension(InkstitchExtension):
         InkstitchExtension.__init__(self, *args, **kwargs)
         for command in self.COMMANDS:
             self.OptionParser.add_option("--%s" % command, type="inkbool")
-
-    @property
-    def symbols_path(self):
-        return os.path.join(get_bundled_dir("symbols"), "inkstitch.svg")
-
-    @property
-    @cache
-    def symbols_svg(self):
-        with open(self.symbols_path) as symbols_file:
-            return inkex.etree.parse(symbols_file)
-
-    @property
-    @cache
-    def symbol_defs(self):
-        return self.symbols_svg.find(SVG_DEFS_TAG)
-
-    @property
-    @cache
-    def defs(self):
-        return self.document.find(SVG_DEFS_TAG)
-
-    def ensure_symbol(self, command):
-        path = "./*[@id='inkstitch_%s']" % command
-        if self.defs.find(path) is None:
-            self.defs.append(deepcopy(self.symbol_defs.find(path)))
-
-    def add_connector(self, symbol, element):
-        # I'd like it if I could position the connector endpoint nicely but inkscape just
-        # moves it to the element's center immediately after the extension runs.
-        start_pos = (symbol.get('x'), symbol.get('y'))
-        end_pos = element.shape.centroid
-
-        path = inkex.etree.Element(SVG_PATH_TAG,
-                                   {
-                                       "id": self.uniqueId("connector"),
-                                       "d": "M %s,%s %s,%s" % (start_pos[0], start_pos[1], end_pos.x, end_pos.y),
-                                       "style": "stroke:#000000;stroke-width:1px;stroke-opacity:0.5;fill:none;",
-                                       CONNECTION_START: "#%s" % symbol.get('id'),
-                                       CONNECTION_END: "#%s" % element.node.get('id'),
-                                       CONNECTOR_TYPE: "polyline",
-
-                                       # l10n: the name of the line that connects a command to the object it applies to
-                                       INKSCAPE_LABEL: _("connector")
-                                   }
-                                   )
-
-        symbol.getparent().insert(0, path)
-
-    def get_command_pos(self, element, index, total):
-        # Put command symbols 30 pixels out from the shape, spaced evenly around it.
-
-        # get a line running 30 pixels out from the shape
-        outline = element.shape.buffer(30).exterior
-
-        # pick this item's spot arond the outline and perturb it a bit to avoid
-        # stacking up commands if they run the extension multiple times
-        position = index / float(total)
-        position += random() * 0.1
-
-        return outline.interpolate(position, normalized=True)
-
-    def remove_legacy_param(self, element, command):
-        if command == "trim" or command == "stop":
-            # If they had the old "TRIM after" or "STOP after" attributes set,
-            # automatically delete them.  THe new commands will do the same
-            # thing.
-            #
-            # If we didn't delete these here, then things would get confusing.
-            # If the user were to delete a "trim" symbol added by this extension
-            # but the "embroider_trim_after" attribute is still set, then the
-            # trim would keep happening.
-
-            attribute = "embroider_%s_after" % command
-
-            if attribute in element.node.attrib:
-                del element.node.attrib[attribute]
-
-    def add_commands(self, element, commands):
-        for i, command in enumerate(commands):
-            self.remove_legacy_param(element, command)
-
-            pos = self.get_command_pos(element, i, len(commands))
-
-            group = inkex.etree.SubElement(element.node.getparent(), SVG_GROUP_TAG,
-                                           {
-                "id": self.uniqueId("group"),
-                INKSCAPE_LABEL: _("Ink/Stitch Command") + ": %s" % get_command_description(command),
-                "transform": get_correction_transform(element.node)
-            }
-            )
-
-            symbol = inkex.etree.SubElement(group, SVG_USE_TAG,
-                                            {
-                                                "id": self.uniqueId("use"),
-                                                XLINK_HREF: "#inkstitch_%s" % command,
-                                                "height": "100%",
-                                                "width": "100%",
-                                                "x": str(pos.x),
-                                                "y": str(pos.y),
-
-                                                # l10n: the name of a command symbol (example: scissors icon for trim command)
-                                                INKSCAPE_LABEL: _("command marker"),
-                                            }
-                                            )
-
-            self.add_connector(symbol, element)
diff --git a/lib/extensions/layer_commands.py b/lib/extensions/layer_commands.py
index 3a746fcf..c124ec95 100644
--- a/lib/extensions/layer_commands.py
+++ b/lib/extensions/layer_commands.py
@@ -1,6 +1,6 @@
 import inkex
 
-from ..commands import LAYER_COMMANDS, get_command_description
+from ..commands import LAYER_COMMANDS, get_command_description, ensure_symbol
 from ..i18n import _
 from ..svg import get_correction_transform
 from ..svg.tags import SVG_USE_TAG, INKSCAPE_LABEL, XLINK_HREF
@@ -21,7 +21,7 @@ class LayerCommands(CommandsExtension):
         correction_transform = get_correction_transform(self.current_layer, child=True)
 
         for i, command in enumerate(commands):
-            self.ensure_symbol(command)
+            ensure_symbol(command)
 
             inkex.etree.SubElement(self.current_layer, SVG_USE_TAG,
                                    {
diff --git a/lib/extensions/object_commands.py b/lib/extensions/object_commands.py
index 47fb361d..d33ab2ba 100644
--- a/lib/extensions/object_commands.py
+++ b/lib/extensions/object_commands.py
@@ -1,8 +1,8 @@
 import inkex
 
-from .commands import CommandsExtension
-from ..commands import OBJECT_COMMANDS
+from ..commands import OBJECT_COMMANDS, add_commands
 from ..i18n import _
+from .commands import CommandsExtension
 
 
 class ObjectCommands(CommandsExtension):
@@ -24,14 +24,11 @@ class ObjectCommands(CommandsExtension):
             inkex.errormsg(_("Please choose one or more commands to attach."))
             return
 
-        for command in commands:
-            self.ensure_symbol(command)
-
         # Each object (node) in the SVG may correspond to multiple Elements of different
         # types (e.g. stroke + fill).  We only want to process each one once.
         seen_nodes = set()
 
         for element in self.elements:
             if element.node not in seen_nodes:
-                self.add_commands(element, commands)
+                add_commands(element, commands)
                 seen_nodes.add(element.node)
diff --git a/lib/svg/__init__.py b/lib/svg/__init__.py
index df76c0d2..0b4a6ee4 100644
--- a/lib/svg/__init__.py
+++ b/lib/svg/__init__.py
@@ -1,4 +1,5 @@
-from .svg import color_block_to_point_lists, render_stitch_plan
-from .units import *
-from .path import apply_transforms, get_node_transform, get_correction_transform, line_strings_to_csp, point_lists_to_csp
 from .guides import get_guides
+from .path import apply_transforms, get_node_transform, get_correction_transform, line_strings_to_csp, point_lists_to_csp
+from .rendering import color_block_to_point_lists, render_stitch_plan
+from .svg import get_document, generate_unique_id
+from .units import *
\ No newline at end of file
diff --git a/lib/svg/realistic_rendering.py b/lib/svg/rendering.py
index 73da3a09..41ed53d7 100644
--- a/lib/svg/realistic_rendering.py
+++ b/lib/svg/rendering.py
@@ -1,8 +1,17 @@
-import simplepath
 import math
 
-from .units import PIXELS_PER_MM
+import inkex
+import simplepath
+import simplestyle
+import simpletransform
+
+from ..i18n import _
 from ..utils import Point
+from ..utils import cache
+from .tags import SVG_GROUP_TAG, INKSCAPE_LABEL, INKSCAPE_GROUPMODE, SVG_PATH_TAG, SVG_DEFS_TAG
+from .units import PIXELS_PER_MM
+from .units import get_viewbox_transform
+
 
 # The stitch vector path looks like this:
 #  _______
@@ -10,7 +19,6 @@ from ..utils import Point
 #
 # It's 0.32mm high, which is the approximate thickness of common machine
 # embroidery threads.
-
 # 1.216 pixels = 0.32mm
 stitch_height = 1.216
 
@@ -128,3 +136,118 @@ def realistic_stitch(start, end):
     simplepath.translatePath(path, stitch_center.x - rotation_center_x, stitch_center.y - rotation_center_y)
 
     return simplepath.formatPath(path)
+
+
+def color_block_to_point_lists(color_block):
+    point_lists = [[]]
+
+    for stitch in color_block:
+        if stitch.trim:
+            if point_lists[-1]:
+                point_lists.append([])
+                continue
+
+        if not stitch.jump and not stitch.color_change:
+            point_lists[-1].append(stitch.as_tuple())
+
+    # filter out empty point lists
+    point_lists = [p for p in point_lists if p]
+
+    return point_lists
+
+
+@cache
+def get_correction_transform(svg):
+    transform = get_viewbox_transform(svg)
+
+    # we need to correct for the viewbox
+    transform = simpletransform.invertTransform(transform)
+    transform = simpletransform.formatTransform(transform)
+
+    return transform
+
+
+def color_block_to_realistic_stitches(color_block, svg):
+    paths = []
+
+    for point_list in color_block_to_point_lists(color_block):
+        if not point_list:
+            continue
+
+        color = color_block.color.visible_on_white.darker.to_hex_str()
+        start = point_list[0]
+        for point in point_list[1:]:
+            paths.append(inkex.etree.Element(
+                SVG_PATH_TAG,
+                {'style': simplestyle.formatStyle(
+                    {
+                        'fill': color,
+                        'stroke': 'none',
+                        'filter': 'url(#realistic-stitch-filter)'
+                    }),
+                 'd': realistic_stitch(start, point),
+                 'transform': get_correction_transform(svg)
+                 }))
+            start = point
+
+    return paths
+
+
+def color_block_to_paths(color_block, svg):
+    paths = []
+    # We could emit just a single path with one subpath per point list, but
+    # emitting multiple paths makes it easier for the user to manipulate them.
+    for point_list in color_block_to_point_lists(color_block):
+        color = color_block.color.visible_on_white.to_hex_str()
+        paths.append(inkex.etree.Element(
+            SVG_PATH_TAG,
+            {'style': simplestyle.formatStyle(
+                {'stroke': color,
+                 'stroke-width': "0.4",
+                 'fill': 'none'}),
+             'd': "M" + " ".join(" ".join(str(coord) for coord in point) for point in point_list),
+             'transform': get_correction_transform(svg),
+             'embroider_manual_stitch': 'true',
+             'embroider_trim_after': 'true',
+             }))
+
+    # no need to trim at the end of a thread color
+    if paths:
+        paths[-1].attrib.pop('embroider_trim_after')
+
+    return paths
+
+
+def render_stitch_plan(svg, stitch_plan, realistic=False):
+    layer = svg.find(".//*[@id='__inkstitch_stitch_plan__']")
+    if layer is None:
+        layer = inkex.etree.Element(SVG_GROUP_TAG,
+                                    {'id': '__inkstitch_stitch_plan__',
+                                     INKSCAPE_LABEL: _('Stitch Plan'),
+                                     INKSCAPE_GROUPMODE: 'layer'})
+    else:
+        # delete old stitch plan
+        del layer[:]
+
+        # make sure the layer is visible
+        layer.set('style', 'display:inline')
+
+    for i, color_block in enumerate(stitch_plan):
+        group = inkex.etree.SubElement(layer,
+                                       SVG_GROUP_TAG,
+                                       {'id': '__color_block_%d__' % i,
+                                        INKSCAPE_LABEL: "color block %d" % (i + 1)})
+        if realistic:
+            group.extend(color_block_to_realistic_stitches(color_block, svg))
+        else:
+            group.extend(color_block_to_paths(color_block, svg))
+
+    svg.append(layer)
+
+    if realistic:
+        defs = svg.find(SVG_DEFS_TAG)
+
+        if defs is None:
+            defs = inkex.etree.SubElement(svg, SVG_DEFS_TAG)
+
+        defs.append(inkex.etree.fromstring(realistic_filter))
diff --git a/lib/svg/svg.py b/lib/svg/svg.py
index 3fceebfb..0ec43f75 100644
--- a/lib/svg/svg.py
+++ b/lib/svg/svg.py
@@ -1,124 +1,19 @@
-import inkex
-import simplestyle
-import simpletransform
-
-from ..i18n import _
 from ..utils import cache
-from .realistic_rendering import realistic_stitch, realistic_filter
-from .tags import SVG_GROUP_TAG, INKSCAPE_LABEL, INKSCAPE_GROUPMODE, SVG_PATH_TAG, SVG_DEFS_TAG
-from .units import get_viewbox_transform
-
-
-def color_block_to_point_lists(color_block):
-    point_lists = [[]]
-
-    for stitch in color_block:
-        if stitch.trim:
-            if point_lists[-1]:
-                point_lists.append([])
-                continue
-
-        if not stitch.jump and not stitch.color_change:
-            point_lists[-1].append(stitch.as_tuple())
-
-    # filter out empty point lists
-    point_lists = [p for p in point_lists if p]
-
-    return point_lists
 
 
 @cache
-def get_correction_transform(svg):
-    transform = get_viewbox_transform(svg)
-
-    # we need to correct for the viewbox
-    transform = simpletransform.invertTransform(transform)
-    transform = simpletransform.formatTransform(transform)
-
-    return transform
-
-
-def color_block_to_realistic_stitches(color_block, svg):
-    paths = []
-
-    for point_list in color_block_to_point_lists(color_block):
-        if not point_list:
-            continue
-
-        color = color_block.color.visible_on_white.darker.to_hex_str()
-        start = point_list[0]
-        for point in point_list[1:]:
-            paths.append(inkex.etree.Element(
-                SVG_PATH_TAG,
-                {'style': simplestyle.formatStyle(
-                    {
-                        'fill': color,
-                        'stroke': 'none',
-                        'filter': 'url(#realistic-stitch-filter)'
-                    }),
-                 'd': realistic_stitch(start, point),
-                 'transform': get_correction_transform(svg)
-                 }))
-            start = point
-
-    return paths
-
-
-def color_block_to_paths(color_block, svg):
-    paths = []
-    # We could emit just a single path with one subpath per point list, but
-    # emitting multiple paths makes it easier for the user to manipulate them.
-    for point_list in color_block_to_point_lists(color_block):
-        color = color_block.color.visible_on_white.to_hex_str()
-        paths.append(inkex.etree.Element(
-            SVG_PATH_TAG,
-            {'style': simplestyle.formatStyle(
-                {'stroke': color,
-                 'stroke-width': "0.4",
-                 'fill': 'none'}),
-             'd': "M" + " ".join(" ".join(str(coord) for coord in point) for point in point_list),
-             'transform': get_correction_transform(svg),
-             'embroider_manual_stitch': 'true',
-             'embroider_trim_after': 'true',
-             }))
-
-    # no need to trim at the end of a thread color
-    if paths:
-        paths[-1].attrib.pop('embroider_trim_after')
-
-    return paths
-
-
-def render_stitch_plan(svg, stitch_plan, realistic=False):
-    layer = svg.find(".//*[@id='__inkstitch_stitch_plan__']")
-    if layer is None:
-        layer = inkex.etree.Element(SVG_GROUP_TAG,
-                                    {'id': '__inkstitch_stitch_plan__',
-                                     INKSCAPE_LABEL: _('Stitch Plan'),
-                                     INKSCAPE_GROUPMODE: 'layer'})
-    else:
-        # delete old stitch plan
-        del layer[:]
-
-        # make sure the layer is visible
-        layer.set('style', 'display:inline')
-
-    for i, color_block in enumerate(stitch_plan):
-        group = inkex.etree.SubElement(layer,
-                                       SVG_GROUP_TAG,
-                                       {'id': '__color_block_%d__' % i,
-                                        INKSCAPE_LABEL: "color block %d" % (i + 1)})
-        if realistic:
-            group.extend(color_block_to_realistic_stitches(color_block, svg))
-        else:
-            group.extend(color_block_to_paths(color_block, svg))
+def get_document(node):
+    return node.getroottree().getroot()
 
-    svg.append(layer)
 
-    if realistic:
-        defs = svg.find(SVG_DEFS_TAG)
+def generate_unique_id(document, prefix="path"):
+    doc_ids = {node.get('id') for node in document.iterdescendants() if 'id' in node.attrib}
 
-        if defs is None:
-            defs = inkex.etree.SubElement(svg, SVG_DEFS_TAG)
+    i = 1
+    while True:
+        new_id = "%s%d" % (prefix, i)
+        if new_id not in doc_ids:
+            break
+        i += 1
 
-        defs.append(inkex.etree.fromstring(realistic_filter))
+    return new_id