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

[FEATURE] Adds inline if and unless keywords #1240

Merged
merged 1 commit into from
Dec 17, 2020

Conversation

pzuraq
Copy link
Member

@pzuraq pzuraq commented Dec 17, 2020

Directly adds the inline versions of the if and unless keywords to
the VM. It does this by adding a new IfInline opcode, which is
combined with a new Not opcode that inverst the condition in the case
of unless. This also allows us to remove the Unless opcode and
rewrite it in terms of the If opcode in general. Since unless is
generally much less common of an opcode, this shouldn't be an issue
performance-wise, and we benefit from having a simpler set of
wireformat/opcodes.

Directly adds the inline versions of the `if` and `unless` keywords to
the VM. It does this by adding a new `IfInline` opcode, which is
combined with a new `Not` opcode that inverst the condition in the case
of `unless`. This also allows us to remove the `Unless` opcode and
rewrite it in terms of the `If` opcode in general. Since `unless` is
generally much less common of an opcode, this shouldn't be an issue
performance-wise, and we benefit from having a simpler set of
wireformat/opcodes.
@lifeart
Copy link
Contributor

lifeart commented Dec 17, 2020

wondering, could we have not instead of unless ?

  • it's widely used outside ember (comparing to unless)
  • easier to understand (for non-english speakers)
  • less symbols to write

downsides:

{{#unless this.foo}} {{/unless}} has to be replaced with {{#if (not this.foo) }} {{/if}},
but, it's bring mutch clarity into logic (IMHO)

one more case - have if-not instead of unless (more clarity)

point is - unless wery "rare" word and adding more mental overhead for non-native speakers, also, used 20 times less in the world, comparing to if not

image

image

@pzuraq
Copy link
Member Author

pzuraq commented Dec 17, 2020

@lifeart yeah, that's actually my preferred usage. (not) is currently in RFC, and the thinking is that this could be how it works under the hood. We need to decide if not is a keyword or a helper first though, so we'll see.

As for removing unless, it already exists in the language so it would need an RFC to remove it. This change means we don't have to pay for it conceptually at least, in the future we could discuss removing.

@rwjblue rwjblue merged commit 1693af6 into master Dec 17, 2020
@rwjblue rwjblue deleted the make-inline-if-and-unless-keywords branch December 17, 2020 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants