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

Logical helpers renaming #714

Closed
lifeart opened this issue Jan 29, 2021 · 9 comments
Closed

Logical helpers renaming #714

lifeart opened this issue Jan 29, 2021 · 9 comments

Comments

@lifeart
Copy link

lifeart commented Jan 29, 2021

This is draft about renaming

unless to if-not
and
neq to not-eq


Why 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

Why not-eq instead of neq ?

not-eq seems more readable for me, comparing to neq

it require less mental parsing, especially for non native speakers and new developers.


Preconditions:

glimmerjs/glimmer-vm#1240 (comment)

Refs:

#560 (comment)
#560 (comment)

cc @rwjblue @pzuraq

@chancancode
Copy link
Member

chancancode commented Jan 30, 2021

Personally, I am 👎 on changing either. It seems fine to have an optional lint rule to forbid {{#unless}} and neq if some people find that less readable (I think it's just a matter of getting used to it – Ruby/Rust has those and IMO it turned out fine), but since you can already compose {{#if (not ...)}} and (not (eq ... )), I don't find a lot of value in adding a - version if that's all it does.

@lifeart
Copy link
Author

lifeart commented Jan 30, 2021

@chancancode compose - increase template complexity, decrease readability and increase opcodes count.

And since we already have this upcodes, I don't think we should offer composition, comparing to update naming

@chriskrycho
Copy link
Contributor

Ruby/Rust has those and IMO it turned out fine

Nit: Rust doesn't (operators, keywords).


On the substance: I also think this is something to handle at the lint level. For example: allowing unless and forbidding unless...else is a common pattern in codebases which do this already.

@NullVoxPopuli
Copy link
Contributor

It's also common to forbid unless altogether. :)

Where I work forbids unless, and it is one of the most unanimous lint decisions we've had. Lol

@lolmaus
Copy link

lolmaus commented Jan 30, 2021

I confirm that unless adds mental overhead and makes it harder to reason about code, especially in complex clauses.

I believe it would be reasonable to update the default linting config to forbid unless entirely. But then, why offer a forbidden helper in the first place?

After all, it's a matter of habit. Developers who are in the habit of using it would vote against the proposal. Developers who are not — will vote in favor. Thus, we should ask ourselves the following question: which habit do we want to endorse? We should consider such aspects as size of the vocabulary, mental overhead, level of nesting, opcode complexity.

@sandstrom
Copy link
Contributor

  • I don't think renaming unless is a good idea.
  • Renaming neq to not-eq makes sense, I think.

However, both of these are fairly subjective.

At least for the helper people can make their own, if they want. There is also linting to disable per project.

This may be one of those things that are easier to handle at the project level, instead of framework level.

@snewcomer
Copy link
Contributor

Btw, we are implementing these as keywords baked into glimmer. In strict mode, these won't be helpers so figuring out our naming will be critical. I'm happy to submit an RFC to rename neq to not-eq. I'll bring this up in our next chat.

glimmerjs/glimmer-vm#1277
snewcomer/glimmer-vm#1
snewcomer/glimmer-vm#2

@gossi
Copy link

gossi commented Mar 9, 2021

Every time I read an {{#unless}} code block, I will just read it without noticing what it does. Just to go back to the beginning of it and read it again

Every time I read an {{#unless}} code block, I will just read it without noticing what it does. Just to go back to the beginning of it and read it again - but this time with if not in mind and now the code is understandable again to me. I noticed that {{unless}} is a big productiviy killer for me.

@wagenet
Copy link
Member

wagenet commented Jul 23, 2022

I'm closing this due to inactivity. This doesn't mean that the idea presented here is invalid, but that, unfortunately, nobody has taken the effort to spearhead it and bring it to completion. Please feel free to advocate for it if you believe that this is still worth pursuing. Thanks!

@wagenet wagenet closed this as completed Jul 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants