Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Added piped helpers #261

Open
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants

techhead commented Oct 1, 2012

Implementation of piped helpers based on the discussion found here: #139

See the new tests.

({
    numbers: [1,2,3,4],
    sum: function() {
        var total = 0;
        for (var i=0,len=this.length; i<len; i++) {
            total += this[i];
        }
        return total;
    }
})
{{numbers|sum}}
({
    timestamp: 1,
    iso8601Date: function() {
        return new Date(this).toISOString();
    }
})
{{timestamp | iso8601Date}}

Helpers are simply lambdas.

{{a | b}} is identical to {{#a}}{{b}}{{/a}} when 'a' is a single value

It is not identical when 'a' is a list. The list will become the context of the lambda to allow for 'reduce' type functions.

techhead commented Oct 1, 2012

I have to make a correction to the statement

{{a | b}} is identical to {{#a}}{{b}}{{/a}} when 'a' is a single value

The two are only equivalent when 'a' is a single truthy value.

Please don't increment the version number. We'll do that when we cut a new stable release.

Owner

techhead replied Oct 2, 2012

Not a problem. I looked for any documentation concerning version numbers in the project, and I could find none.

Thanks for looking, by the way.

Instead of coercing the tag value to be "hi.world" in the parser test, couldn't we just split on /\s*\|\s*/ here?

Owner

techhead replied Oct 2, 2012

Yes, we could. However, spaces are not part of valid tag names. You'll notice that for dot notation, the code simply splits on '.'. Before stripping the spaces, dot notation was broken if there was a space on either side.

You're right, spaces are not part of valid tag names. However, I'd prefer to make the parser forgiving in this case and allow spaces for those who want to use them. You're also right that if we split on space + vertical bar for filters that we should do the same for dot notation.

Owner

techhead replied Oct 2, 2012

Well, you probably have a point in that this would be a breaking change for those using spaces -- if any. My thinking was to take as much of the performance hit as possible up front in the parsing phase, but this may be a premature optimization. At any rate, probably better to keep from breaking anyone... Of course, this still will if they were using pipes in their tag names.

If we retain the whitespace as part of the token's value and simply split on whitespace + vertical bar as I suggest above then this becomes unnecessary and the diff is much smaller.

Owner

techhead replied Oct 2, 2012

Please see my comment on your previous comment. I am fine with this suggestion. However, I think that we'd need to do this for fixing dot notation as well.

Owner

techhead commented on mustache.js in bca914c Oct 2, 2012

This could be problematic depending on what you expect. If view is not truthy, then view will "revert" to the context at the top of the stack. However, consider an example where zero has an actual meaning that you may want to have the helper act on (such as a timestamp).

The following code would probably be a better replacement:

value = value.call(typeof view === 'undefined' ? this.view : view);

techhead commented Oct 2, 2012

@mjijackson I've incorporated the changes that we talked about.

@techhead techhead referenced this pull request Oct 2, 2012

Open

Implement Helpers. #139

groue commented Nov 2, 2012

There have been a debate at mustache/spec, but it looks dead now: mustache/spec#41

GRMustache, the Objective-C implementation, has chosen the parenthesis syntax, {{ f(x) }}, for "filters" : https://github.com/groue/GRMustache/blob/master/Guides/filters.md. Highlights are the possibility to build other expressions on top of filtered values, like {{ last(people).name }}, and filters that take several arguments, as in {{ localize(date, format) }}.

I'd be happy that janl/mustache.js considers this option, regardless of the trend for pipes today. You can read arguments against pipes at https://github.com/groue/GRMustache/blob/master/Articles/WhyMustacheFilters.md.

Since very few Mustache implementations have shipped today with piped helpers, I think it's still time to prevent this bad idea to spread. @janl, @mjijackson, what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment