From 7328825204c3ab0e160cd64451a28d3a7026ee91 Mon Sep 17 00:00:00 2001
From: Jeff Avallone <jeff.avallone@gmail.com>
Date: Fri, 13 Mar 2015 20:18:46 -0400
Subject: [PATCH] Changing when subexpression numbers are generated

Generating subexp numbers during setup was leading to nested expressions
being numbered in reverse order.

Fixes #7
---
 spec/parser/javascript/subexp_spec.js | 47 +++++++++++++++++++--------
 src/js/parser/javascript/subexp.js    | 27 +++++++++++----
 2 files changed, 54 insertions(+), 20 deletions(-)

diff --git a/spec/parser/javascript/subexp_spec.js b/spec/parser/javascript/subexp_spec.js
index 2f15c67..0a0a3b9 100644
--- a/spec/parser/javascript/subexp_spec.js
+++ b/spec/parser/javascript/subexp_spec.js
@@ -12,25 +12,17 @@ describe('parser/javascript/subexp.js', function() {
 
   _.forIn({
     '(test)': {
-      label: 'group #1',
-      regexp: jasmine.objectContaining({ textValue: 'test' }),
-      state: { groupCounter: 2 }
+      regexp: jasmine.objectContaining({ textValue: 'test' })
     },
     '(?=test)': {
-      label: 'positive lookahead',
-      regexp: jasmine.objectContaining({ textValue: 'test' }),
-      state: { groupCounter: 1 }
+      regexp: jasmine.objectContaining({ textValue: 'test' })
     },
     '(?!test)': {
-      label: 'negative lookahead',
-      regexp: jasmine.objectContaining({ textValue: 'test' }),
-      state: { groupCounter: 1 }
+      regexp: jasmine.objectContaining({ textValue: 'test' })
     },
     '(?:test)': {
-      label: '',
       regexp: jasmine.objectContaining({ textValue: 'test' }),
-      proxy: jasmine.objectContaining({ textValue: 'test' }),
-      state: { groupCounter: 1 }
+      proxy: jasmine.objectContaining({ textValue: 'test' })
     }
   }, (content, str) => {
     it(`parses "${str}" as a Subexp`, function() {
@@ -71,9 +63,9 @@ describe('parser/javascript/subexp.js', function() {
       this.renderDeferred = Q.defer();
 
       this.node = new javascript.Parser('(test)').__consume__subexp();
-      this.node.label = 'example label';
       this.node.regexp = jasmine.createSpyObj('regexp', ['render']);
       this.node.container = jasmine.createSpyObj('container', ['addClass', 'group']);
+      spyOn(this.node, 'label').and.returnValue('example label')
 
       this.node.regexp.render.and.returnValue(this.renderDeferred.promise);
     });
@@ -96,4 +88,33 @@ describe('parser/javascript/subexp.js', function() {
 
   });
 
+  describe('#label', function() {
+
+    _.forIn({
+      '(test)': {
+        label: 'group #1',
+        groupCounter: 2
+      },
+      '(?=test)': {
+        label: 'positive lookahead',
+        groupCounter: 1
+      },
+      '(?!test)': {
+        label: 'negative lookahead',
+        groupCounter: 1
+      },
+      '(?:test)': {
+        label: '',
+        groupCounter: 1
+      }
+    }, (data, str) => {
+      it(`generates the correct label for "${str}"`, function() {
+        var node = new javascript.Parser(str).__consume__subexp();
+        expect(node.label()).toEqual(data.label);
+        expect(node.state.groupCounter).toEqual(data.groupCounter);
+      });
+    });
+
+  });
+
 });
diff --git a/src/js/parser/javascript/subexp.js b/src/js/parser/javascript/subexp.js
index bc3c168..7ac1042 100644
--- a/src/js/parser/javascript/subexp.js
+++ b/src/js/parser/javascript/subexp.js
@@ -25,24 +25,37 @@ export default {
   },
 
   _render() {
+    // NOTE: this.label() MUST be called here, in _render and before any child
+    // nodes are rendered. This is to keep the group numbers in the correct
+    // order.
+    var label = this.label();
+
     return this.regexp.render(this.container.group())
       .then(() => {
-        return this.renderLabeledBox(this.label, this.regexp, {
+        return this.renderLabeledBox(label, this.regexp, {
           padding: 10
         });
       });
   },
 
-  setup() {
-    if (_.has(this.labelMap, this.properties.capture.textValue)) {
-      this.label = this.labelMap[this.properties.capture.textValue];
-    } else {
-      this.label = 'group #' + (this.state.groupCounter++);
+  label() {
+    if (typeof this._label === 'undefined') {
+      if (_.has(this.labelMap, this.properties.capture.textValue)) {
+        this._label = this.labelMap[this.properties.capture.textValue];
+      } else {
+        this._label = 'group #' + (this.state.groupCounter++);
+      }
     }
 
+    return this._label;
+  },
+
+  setup() {
+    // NOTE: DO NOT call this.label() in setup. It will lead to groups being
+    // numbered in reverse order
     this.regexp = this.properties.regexp;
 
-    if (!this.label) {
+    if (this.properties.capture.textValue == '?:') {
       this.proxy = this.regexp;
     }
   }