From 06a7ffc110b5679ca6ceee3549c1fec81fe2f0fe Mon Sep 17 00:00:00 2001
From: Jeff Avallone <jeff.avallone@gmail.com>
Date: Thu, 23 Apr 2015 20:03:25 -0400
Subject: [PATCH] Moving skip and loop line calculation into Repeat nodes

This code seems more at home in Repeat instead of MatchFragment since
Repeat knows about the dimensions of the lines for the contentPosition
value.
---
 spec/parser/javascript/match_fragment_spec.js | 89 ++-----------------
 spec/parser/javascript/repeat_spec.js         | 74 +++++++++++++++
 src/js/parser/javascript/match_fragment.js    | 43 +--------
 src/js/parser/javascript/repeat.js            | 39 ++++++++
 4 files changed, 120 insertions(+), 125 deletions(-)

diff --git a/spec/parser/javascript/match_fragment_spec.js b/spec/parser/javascript/match_fragment_spec.js
index 2ae23b4..7b280e2 100644
--- a/spec/parser/javascript/match_fragment_spec.js
+++ b/spec/parser/javascript/match_fragment_spec.js
@@ -77,11 +77,11 @@ describe('parser/javascript/match_fragment.js', function() {
       this.node.content.render.and.returnValue(this.renderDeferred.promise);
 
       this.node.repeat = {
-        contentPosition: 'example position'
+        contentPosition: 'example position',
+        skipPath: jasmine.createSpy('skipPath').and.returnValue('skip path'),
+        loopPath: jasmine.createSpy('loopPath').and.returnValue('loop path')
       };
 
-      spyOn(this.node, 'skipPath').and.returnValue('skip path');
-      spyOn(this.node, 'loopPath').and.returnValue('loop path');
       spyOn(this.node, 'loopLabel');
     });
 
@@ -107,8 +107,8 @@ describe('parser/javascript/match_fragment.js', function() {
       it('renders a skip path and loop path', function(done) {
         this.node._render()
           .then(() => {
-            expect(this.node.skipPath).toHaveBeenCalledWith('content bbox');
-            expect(this.node.loopPath).toHaveBeenCalledWith('content bbox');
+            expect(this.node.repeat.skipPath).toHaveBeenCalledWith('content bbox');
+            expect(this.node.repeat.loopPath).toHaveBeenCalledWith('content bbox');
             expect(this.node.container.path).toHaveBeenCalledWith('skip pathloop path');
             done();
           });
@@ -126,85 +126,6 @@ describe('parser/javascript/match_fragment.js', function() {
 
   });
 
-  describe('#skipPath', function() {
-
-    beforeEach(function() {
-      this.node = new javascript.Parser('a').__consume__match_fragment();
-
-      this.node.repeat = {};
-
-      this.box = {
-        y: 11,
-        ay: 22,
-        width: 33
-      };
-    });
-
-    it('returns nothing when there is no skip', function() {
-      this.node.repeat.hasSkip = false;
-      expect(this.node.skipPath(this.box)).toEqual([]);
-    });
-
-    it('returns a path when there is a skip', function() {
-      this.node.repeat.hasSkip = true;
-      this.node.repeat.greedy = true;
-      expect(this.node.skipPath(this.box)).toEqual([
-        'M0,22q10,0 10,-10v-1q0,-10 10,-10h23q10,0 10,10v1q0,10 10,10'
-      ]);
-    });
-
-    it('returns a path with arrow when there is a non-greedy skip', function() {
-      this.node.repeat.hasSkip = true;
-      this.node.repeat.greedy = false;
-      expect(this.node.skipPath(this.box)).toEqual([
-        'M0,22q10,0 10,-10v-1q0,-10 10,-10h23q10,0 10,10v1q0,10 10,10',
-        'M10,7l5,5m-5,-5l-5,5'
-      ]);
-    });
-
-
-  });
-
-  describe('#loopPath', function() {
-
-    beforeEach(function() {
-      this.node = new javascript.Parser('a').__consume__match_fragment();
-
-      this.node.repeat = {};
-
-      this.box = {
-        x: 11,
-        x2: 22,
-        ay: 33,
-        y2: 44,
-        width: 55
-      };
-    });
-
-    it('returns nothing when there is no loop', function() {
-      this.node.repeat.hasLoop = false;
-      expect(this.node.loopPath(this.box)).toEqual([]);
-    });
-
-    it('returns a path when there is a loop', function() {
-      this.node.repeat.hasLoop = true;
-      this.node.repeat.greedy = false;
-      expect(this.node.loopPath(this.box)).toEqual([
-        'M11,33q-10,0 -10,10v1q0,10 10,10h55q10,0 10,-10v-1q0,-10 -10,-10'
-      ]);
-    });
-
-    it('returns a path with arrow when there is a greedy loop', function() {
-      this.node.repeat.hasLoop = true;
-      this.node.repeat.greedy = true;
-      expect(this.node.loopPath(this.box)).toEqual([
-        'M11,33q-10,0 -10,10v1q0,10 10,10h55q10,0 10,-10v-1q0,-10 -10,-10',
-        'M32,48l5,-5m-5,5l-5,-5'
-      ]);
-    });
-
-  });
-
   describe('#loopLabel', function() {
 
     beforeEach(function() {
diff --git a/spec/parser/javascript/repeat_spec.js b/spec/parser/javascript/repeat_spec.js
index da7cbea..fa803de 100644
--- a/spec/parser/javascript/repeat_spec.js
+++ b/spec/parser/javascript/repeat_spec.js
@@ -253,4 +253,78 @@ describe('parser/javascript/repeat.js', function() {
 
   });
 
+  describe('#skipPath', function() {
+
+    beforeEach(function() {
+      this.node = new javascript.Parser('*').__consume__repeat();
+
+      this.box = {
+        y: 11,
+        ay: 22,
+        width: 33
+      };
+    });
+
+    it('returns nothing when there is no skip', function() {
+      this.node.hasSkip = false;
+      expect(this.node.skipPath(this.box)).toEqual([]);
+    });
+
+    it('returns a path when there is a skip', function() {
+      this.node.hasSkip = true;
+      this.node.greedy = true;
+      expect(this.node.skipPath(this.box)).toEqual([
+        'M0,22q10,0 10,-10v-1q0,-10 10,-10h23q10,0 10,10v1q0,10 10,10'
+      ]);
+    });
+
+    it('returns a path with arrow when there is a non-greedy skip', function() {
+      this.node.hasSkip = true;
+      this.node.greedy = false;
+      expect(this.node.skipPath(this.box)).toEqual([
+        'M0,22q10,0 10,-10v-1q0,-10 10,-10h23q10,0 10,10v1q0,10 10,10',
+        'M10,7l5,5m-5,-5l-5,5'
+      ]);
+    });
+
+  });
+
+  describe('#loopPath', function() {
+
+    beforeEach(function() {
+      this.node = new javascript.Parser('*').__consume__repeat();
+
+      this.box = {
+        x: 11,
+        x2: 22,
+        ay: 33,
+        y2: 44,
+        width: 55
+      };
+    });
+
+    it('returns nothing when there is no loop', function() {
+      this.node.hasLoop = false;
+      expect(this.node.loopPath(this.box)).toEqual([]);
+    });
+
+    it('returns a path when there is a loop', function() {
+      this.node.hasLoop = true;
+      this.node.greedy = false;
+      expect(this.node.loopPath(this.box)).toEqual([
+        'M11,33q-10,0 -10,10v1q0,10 10,10h55q10,0 10,-10v-1q0,-10 -10,-10'
+      ]);
+    });
+
+    it('returns a path with arrow when there is a greedy loop', function() {
+      this.node.hasLoop = true;
+      this.node.greedy = true;
+      expect(this.node.loopPath(this.box)).toEqual([
+        'M11,33q-10,0 -10,10v1q0,10 10,10h55q10,0 10,-10v-1q0,-10 -10,-10',
+        'M32,48l5,-5m-5,5l-5,-5'
+      ]);
+    });
+
+  });
+
 });
diff --git a/src/js/parser/javascript/match_fragment.js b/src/js/parser/javascript/match_fragment.js
index e200613..cd23ecb 100644
--- a/src/js/parser/javascript/match_fragment.js
+++ b/src/js/parser/javascript/match_fragment.js
@@ -38,8 +38,8 @@ export default {
 
         // Add skip or repeat paths to the container.
         paths = _.flatten([
-          this.skipPath(box),
-          this.loopPath(box)
+          this.repeat.skipPath(box),
+          this.repeat.loopPath(box)
         ]);
 
         this.container.prepend(
@@ -49,45 +49,6 @@ export default {
       });
   },
 
-  // Returns the path spec to render the line that skips over the content for
-  // fragments that are optionally matched.
-  skipPath(box) {
-    var paths = [];
-
-    if (this.repeat.hasSkip) {
-      let vert = Math.max(0, box.ay - box.y - 10),
-          horiz = box.width - 10;
-
-      paths.push(`M0,${box.ay}q10,0 10,-10v${-vert}q0,-10 10,-10h${horiz}q10,0 10,10v${vert}q0,10 10,10`);
-
-      // When the repeat is not greedy, the skip path gets a preference arrow.
-      if (!this.repeat.greedy) {
-        paths.push(`M10,${box.ay - 15}l5,5m-5,-5l-5,5`);
-      }
-    }
-
-    return paths;
-  },
-
-  // Returns the path spec to render the line that repeats the content for
-  // fragments that are matched more than once.
-  loopPath(box) {
-    var paths = [];
-
-    if (this.repeat.hasLoop) {
-      let vert = box.y2 - box.ay - 10;
-
-      paths.push(`M${box.x},${box.ay}q-10,0 -10,10v${vert}q0,10 10,10h${box.width}q10,0 10,-10v${-vert}q0,-10 -10,-10`);
-
-      // When the repeat is greedy, the loop path gets the preference arrow.
-      if (this.repeat.greedy) {
-        paths.push(`M${box.x2 + 10},${box.ay + 15}l5,-5m-5,5l-5,-5`);
-      }
-    }
-
-    return paths;
-  },
-
   // Renders label for the loop path indicating how many times the content may
   // be matched.
   loopLabel() {
diff --git a/src/js/parser/javascript/repeat.js b/src/js/parser/javascript/repeat.js
index c201e9e..e753d02 100644
--- a/src/js/parser/javascript/repeat.js
+++ b/src/js/parser/javascript/repeat.js
@@ -47,6 +47,45 @@ export default {
     }
   },
 
+  // Returns the path spec to render the line that skips over the content for
+  // fragments that are optionally matched.
+  skipPath(box) {
+    var paths = [];
+
+    if (this.hasSkip) {
+      let vert = Math.max(0, box.ay - box.y - 10),
+          horiz = box.width - 10;
+
+      paths.push(`M0,${box.ay}q10,0 10,-10v${-vert}q0,-10 10,-10h${horiz}q10,0 10,10v${vert}q0,10 10,10`);
+
+      // When the repeat is not greedy, the skip path gets a preference arrow.
+      if (!this.greedy) {
+        paths.push(`M10,${box.ay - 15}l5,5m-5,-5l-5,5`);
+      }
+    }
+
+    return paths;
+  },
+
+  // Returns the path spec to render the line that repeats the content for
+  // fragments that are matched more than once.
+  loopPath(box) {
+    var paths = [];
+
+    if (this.hasLoop) {
+      let vert = box.y2 - box.ay - 10;
+
+      paths.push(`M${box.x},${box.ay}q-10,0 -10,10v${vert}q0,10 10,10h${box.width}q10,0 10,-10v${-vert}q0,-10 -10,-10`);
+
+      // When the repeat is greedy, the loop path gets the preference arrow.
+      if (this.greedy) {
+        paths.push(`M${box.x2 + 10},${box.ay + 15}l5,-5m-5,5l-5,-5`);
+      }
+    }
+
+    return paths;
+  },
+
   setup() {
     this.minimum = this.properties.spec.minimum;
     this.maximum = this.properties.spec.maximum;