Program location info slightly off

[mmun]

The location info of a program that belongs to a block is slightly off. Specifically, the loc.start for both the main and inverse are set to the beginning of the block's mustache rather than the end.

Here's a JSBin illustrating it for 3.0.3: http://jsbin.com/yemadanida/1/edit?html,js,console

I also think loc.source should be set to null instead of undefined because it's JSONifyable and it's what's documented anyways.

[kpdecker]

Re: loc, it sounds like we should change that.

Re: source, null vs. undefined is a pet peeve of mine (See the fun with express handlebars defaults in the last release). Everyone has their own definition and nuances of what the things mean. Generally I prefer the route that creates the least amount of code, i.e. implicit undefineds vs. explicit nulls. Should we change the docs to | undefined | null to make it clear that code should safely handle both?

[mmun]

@kpdecker Re: source, yeah that's fine, though I'd hoped it the AST would be valid JSON (undefined isn't a valid value in JSON).

[kpdecker]

Undefined is perfectly valid json. {} has every field name as undefined ;)

On Fri, May 22, 2015 at 1:56 PM Martin Muñoz [email protected]
wrote:

@kpdecker https://github.com/kpdecker Re: source, yeah that's fine,
though I'd hoped it the AST would be valid JSON (undefined isn't a valid
value in JSON).

—
Reply to this email directly or view it on GitHub
#1024 (comment)
.

[kpdecker]

Test case:

  it('GH1024 - should track program location properly', function() {
    var p = Handlebars.parse('\n'
      + '  {{#if foo}}\n'
      + '    {{bar}}\n'
      + '       {{else}}    {{baz}}\n'
      + '\n'
      + '     {{/if}}\n'
      + '    ');

    console.log(JSON.stringify(p));

    // We really need a deep equals but for now this should be stable...
    equals(JSON.stringify(p.loc), JSON.stringify({
      start: { line: 1, column: 0 },
      end: { line: 7, column: 4 }
    }));
    equals(JSON.stringify(p.body[1].program.loc), JSON.stringify({
      start: { line: 2, column: 13 },
      end: { line: 4, column: 7 }
    }));
    equals(JSON.stringify(p.body[1].inverse.loc), JSON.stringify({
      start: { line: 4, column: 15 },
      end: { line: 6, column: 5 }
    }));
  });

[kpdecker]

@mmun would you argue that this test case is incorrect now? eee2c4d#diff-162dcff9d8009bd8efba3ccd7ee514d9R240

[mmun]

@kpdecker Yes, I would argue they are incorrect.

The block helper program node test should be testColumns(program, 3, 5, 31, 5);.
The block helper inverse node test should be testColumns(inverse, 5, 7, 13, 0);.
The block helper node test itself is 👍.