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

Introduce apply as alternative to := #230

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lolgab
Copy link
Member

@lolgab lolgab commented Dec 10, 2021

Laminar introduced the convention (I'm not sure if this was already available before somewhere else) to replace the attribute := value syntax with attribute(value).
I find this syntax much easier to use, and it would be nice if Scalatags supported it.

@lolgab lolgab marked this pull request as ready for review December 10, 2021 15:27
@lihaoyi
Copy link
Member

lihaoyi commented Dec 10, 2021

TBH I've thought about this before. It would be a major syntactic change, but the change would only by skin-deep. I'm curious what other people think about it? This is definitely a preference thing, and I'd be happy to go along with whatever the overall consensus is.

@lolgab perhaps you could flesh out the PR description a bit: explain why you're doing this, what the pros and cons are, provide a few more examples, etc.. Then I'll see if I can get folks to look at this PR to see what they think in aggregate?

@raquo
Copy link

raquo commented Dec 11, 2021

My two cents:

In Laminar we use := throughout documentation because that method name is easier to refer to in writing (if we're talking about apply, we need to clarify which apply, but there is only one :=), and it's easier for people learning the library to go-to-definition of this method in their IDE to see what it's doing (in contrast, the apply method name does not actually appear in your code if you use it, so it's not obvious how you can get to its method definition in the IDE).

In Laminar docs there's a short section that recommends that people use apply, but otherwise it's left up to users. I think it's good to give users the option, considering how dramatic the visual change is, and how cheap it is to implement.

Personally I vastly prefer typing shift+9 to space-shift-colon-equals-space, and I like the resulting clean look too.

@raquo
Copy link

raquo commented Dec 11, 2021

Oh, one other aspect of using apply syntax is that usages of user-defined helpers which return modifiers, such as myFontVariant(brand.darkThemed), look more first-class, similar to library-built-ins like key(attr). This gives a more uniform look. Conversely, when you use the := method for built-ins, user-defined helpers like this stand out visually (unless users take the effort to define classes / extension methods to match the := syntax, which is annoying).

I'm not saying which side of this aspect is the winner, I'm not sure myself, just something I've noticed in a similar context of Laminar's --> <-- methods (which have no apply equivalent, unlike :=... maybe they should, but it starts feeling too magicky at this point).

@seoethereal
Copy link
Contributor

Seems nice but please do not deprecate the := yet!

@Bathtor
Copy link

Bathtor commented Dec 17, 2021

Seems nice but please do not deprecate the := yet!

Please don't deprecate := ever.

The apply syntax is nice, and I'm happy for people to use it who like that.
But for myself, I got enough parenthesis nesting in my code without it. Any more and I'll feel like I'm writing Lisp :D

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

Successfully merging this pull request may close these issues.

5 participants