Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add {{elseif}} sugar syntax #50

Closed
wants to merge 7,343 commits into from
Closed

Add {{elseif}} sugar syntax #50

wants to merge 7,343 commits into from

Conversation

@akanix42
Copy link

akanix42 commented Jul 28, 2016

Implements #39.

{{#if counterIsAt 2}}
    test
{{elseif counterIsAt 3}}
    test3
{{elseif counterIsAt 4}}
    test4
{{else}}
    test5
{{/if}}

Working example here: https://github.com/nathantreid/blaze-test-issue-39

@mitar
Copy link
Collaborator

mitar commented Sep 8, 2016

@nathantreid: This is great work. Can you please resolve merge conflicts? And add also all changes to documentation (spacebars API, mention this also somewhere in guide, blaze itself, and so on)?

We have now documentation in this repository, so you can just update this pull request.

@mitar
Copy link
Collaborator

mitar commented Sep 9, 2016

BTW, Handlebars seems to have else if.

See also example in documentation. It seems the syntax is:

{{else if isInactive}}

And also:

{{else unless isInactive}}

Or in general it seems it can be {{else <any block helper>}}.

It is not necessary to use the same helper in subsequent calls, the unless helper could be used in the else portion as with any other helper. When the helper values are different, the closing mustache should match the opening helper name.

While I dislike this syntax, I see benefit of having it match Handlebars, and benefit of supporting any block helper. Implementation might be a bit trickier to implement, but this sugar is easy to do:

{{#if foo}}

{{else something bar}}

{{/if}}

Becomes:

{{#if foo}}

{{else}}
  {{#something bar}}

  {{/something}}
{{/if}}

@nathantreid, could you implement it this way?

@stubailo
Copy link
Contributor

stubailo commented Sep 9, 2016

Yikes.... that seems kind of odd. Are we sure we want to think about handlebars at all? Honestly Handlebars doesn't seem that well designed to start with.

stubailo added 2 commits Sep 9, 2016
@mitar
Copy link
Collaborator

mitar commented Sep 9, 2016

For this particular feature I think I would. I would change what this means inside a template, but for syntax I would try to stick with Handlebars.

@mitar
Copy link
Collaborator

mitar commented Sep 9, 2016

Also, this syntax seems to map nicely to the sugar we are trying to make.

@akanix42
Copy link
Author

akanix42 commented Sep 9, 2016

I also think that the {{else if}} syntax looks odd, perhaps we could put that up for a vote?
I personally don't have any desire to match Handlebars going forward, unless of course it's because the syntax actually makes a lot of sense on its own.

@mitar
Copy link
Collaborator

mitar commented Sep 9, 2016

It is not just a syntax here, but also a feature. So that you can use any block helper in this way. elseif does not support that. I think that a feature of else <blockname> is a good one. I use quite some custom block helpers (Blaze Components add extra power to them).

@bengott

This comment has been minimized.

Copy link
Contributor

bengott commented on 6af0a1b Sep 14, 2016

'Edit on GitHub' links are broken

I'm not very familiar with Hexo config files, but it looks like github_repo and content_root are used to generate the orange 'Edit on GitHub' link buttons. After looking at the history, it appears that these two lines have changed a few times in the process of moving things around.

I had a PR in the works, but given my lack of experience with Hexo, I figured it would be best to comment here first. I'm assuming that changing content_root: 'site' to content_root: 'site/source'will fix the links. Am I correct? @mitar @alexandesigner

This comment has been minimized.

Copy link
Collaborator

mitar replied Sep 14, 2016

Yea, I am not sure what was the change here, @stubailo?

This comment has been minimized.

Copy link
Contributor Author

stubailo replied Sep 14, 2016

ohh, yeah perhaps that is better. It didn't work before I made this change either though. I thought this would be it but it wasn't.

This comment has been minimized.

Copy link
Collaborator

mitar replied Sep 14, 2016

:-)

@bengott: Thanks for looking into this. I suggest you simply do your changes locally and then run Hexo locally and see what is the result and if links work. You can see how to do that in README in site directory.

This comment has been minimized.

Copy link
Contributor

bengott replied Sep 14, 2016

So running Hexo locally was easier than I thought... :) My suggested changes fixed the links as expected. Submitted PR #99, and @mitar already merged it.

@tmeasday tmeasday force-pushed the meteor:master branch from dbe79ef to 0b55365 Sep 14, 2016
@mitar mitar modified the milestone: v2.2 Sep 14, 2016
@mitar
Copy link
Collaborator

mitar commented Sep 26, 2016

@nathantreid, I really love your work here and I think many people would like to see this in. But I also really like that the Handlebars syntax work with arbitrary block helpers. I think that block helpers are really powerful feature of Blaze and with allowing such chaining of them it would be really powerful. Currently block helpers were a bit strange to use because what if you had only two cases, but now you can potentially get multiple cases supported, not just true/false.

Could you please update the pull request to support arbitrary block helpers?

@mitar
Copy link
Collaborator

mitar commented Sep 26, 2016

So something like this could then work:

{{#tab title="Foo"}}

{{else tab title="Bar"}}

{{else tab title="Baz"}}

{{/tab}}

And have a block helper which renders dynamically tabs, opens/closes them, etc.

@akanix42
Copy link
Author

akanix42 commented Sep 27, 2016

I'd be happy too, but I'm swamped right now IRL. If someone else wants to have a go at it, be my guest. Otherwise, I'll try to get to it as soon as I can.

@mitar
Copy link
Collaborator

mitar commented Sep 27, 2016

No worries. I just wanted to make sure we have a path forward.

@mitar
Copy link
Collaborator

mitar commented Nov 15, 2016

Any progress here? Anyone else wants to tackle this?

@kulttuuri
Copy link

kulttuuri commented Dec 4, 2016

This would be fantastic feature to have. @nathantreid still busy with things IRL? :)

@mitar mitar force-pushed the meteor:master branch from 239be86 to d95e9f4 Dec 30, 2016
@mitar
Copy link
Collaborator

mitar commented Dec 30, 2016

I cleaned master branch to remove all Meteor history (#112). This means the pull request should be also updated to new history to not base it on old commits. Sorry for this extra work.

@mitar
Copy link
Collaborator

mitar commented Jan 1, 2017

With 2fa3cab I implemented the Handlebars syntax for chaining block tags. I am closing this pull request in favor of that.

@mitar mitar closed this Jan 1, 2017
@mitar
Copy link
Collaborator

mitar commented Jan 12, 2017

Released as 2.3.0. You have to update templating package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

You can’t perform that action at this time.