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

[JavaScript] [RFC] Removing special handling of dollar signs. #1337

Open
Thom1729 opened this issue Dec 3, 2017 · 9 comments
Open

[JavaScript] [RFC] Removing special handling of dollar signs. #1337

Thom1729 opened this issue Dec 3, 2017 · 9 comments
Labels

Comments

@Thom1729
Copy link
Collaborator

Thom1729 commented Dec 3, 2017

JavaScript allows dollar signs in identifiers. Dollar signs have no special meaning; they're just like any other identifier character. The popular jQuery framework names its global “namespace” variable $. An old version of this syntax marked identifiers consisting of a single dollar sign keyword.other.jquery. A later update corrected this, marking that identifier normally. This proved controversial, as some users liked the old highlighting (#141). Consequently, the syntax was modified to do both — leading dollar signs in identifiers are marked as punctuation.dollar, and identifiers beginning with dollar signs are marked as variable.other.dollar or variable.other.dollar.only.

Now, this may be convenient for users of a specific framework, but as implemented it's problematic for several reasons:

  • It's “cruft”: special-case logic for a specific third-party framework. We have been moving away from that, and for good reason.
  • The $ is marked as punctuation, but this is spurious and arguably incorrect; the language makes no such distinction.
  • It interferes with other variable subscopes. For example, as it stands all variables are marked variable.other.readwrite unless they begin with a $, in which case they're marked variable.other.dollar.
  • It bloats the syntax definition. The hassle it introduces when editing the syntax is totally out of proportion to its usefulness. At present, I count eleven contexts that implement special-case logic to handle dollar signs in identifiers, not counting contexts that merely depend upon those. Removing the jQuery-specific logic would trim dozens of lines of dubious code.
  • It's not self-consistent. There are many places that highlight identifiers that do not special-case dollar signs.

There are several potential solutions to this:

  1. Preserve the behavior as-is. Clearly, I'm not a fan.
  2. Preserve the behavior, make it consistent thoughout the syntax, and maintain it forever. Blech.
  3. Nuke the behavior entirely. Elegant, though possibly overkill.
  4. Nuke the behavior in general, but highlight $ as support.function.jquery. This is in line with what we do native DOM and node.js constants. We'd still be implementing nonstandard third-party functionality in the core syntax, but it's not so bad as these things go.
  5. Nuke the behavior in general, but preserve it as-is only for variables in expressions. This is inelegant and inconsistent, but it would preserve the existing behavior in common cases without metastasizing throughout the syntax.

Obviously, I favor options 3 and 4. With option 3 in particular, it would be easy for a third party to extend the syntax to handle jQuery however they like, which I think is a good general answer to this sort of problem. I will even volunteer to do this myself. (At the expense of sounding like a one-trick pony, I feel obliged to note that my YAML Macros package is ideally suited to this sort of extension. This itself may be an argument for moving special dollar-sign handling to a third-party package that could take advantage of it.)

Thoughts, objections, alternative suggestions?

@keith-hall
Copy link
Collaborator

I vote for 4, I think including some popular third party framework methods/identifiers has its benefits in some circumstances. for example where do you draw the line otherwise? Technically (though correct me if I'm wrong) window, for example, isn't part of the ECMAScript specification itself, but part of the common/standardized browser interface API, and would have no meaning when the JS is not run in a browser context. So I think we could keep $ function special casing as well as other common context-specific identifiers.

@Thom1729
Copy link
Collaborator Author

Thom1729 commented Dec 3, 2017

I've submitted PR #1339 to illustrate approach (4). We save 60 net lines in the syntax definition, which is ~3.6% of the original file size.

@Thom1729
Copy link
Collaborator Author

Thom1729 commented Dec 3, 2017

Technically (though correct me if I'm wrong) window, for example, isn't part of the ECMAScript specification itself, but part of the common/standardized browser interface API, and would have no meaning when the JS is not run in a browser context.

In a perfect world, all of these would be handled by syntax extensions and you could customize the syntax with a simple sublime-settings file. Alas, we might be waiting weeks on such functionality. ;-)

@wbond
Copy link
Member

wbond commented Dec 4, 2017

I don't have any intention of considering this at the current time, just due to the huge amount of feedback and frustrated users the last time we did major work on the JavaScript syntax and highlighting of $ changed.

@Thom1729
Copy link
Collaborator Author

Thom1729 commented Dec 4, 2017

The problem at the present time is that the current highlighting is inconsistent. In some places it looks for the dollar sign, and in other places it doesn't (import/export names, class and function names, inherited class names, function parameters, method names). Is it a bug that the dollar sign is not highlighted in these places? If not, how do we decide where it should be highlighted and where it shouldn't? The motivation for this discussion is that I'm implementing modern JavaScript binding patterns. Do I need to add new implementations of the dollar sign highlighting to that issue?

I've reviewed the discussions from build 3103, and I can see that quite a few users were upset that identifiers beginning with the dollar sign were highlighted normally. I agree that we don't want to leave those users out to dry. On the other hand, the lack of a clean, consistent solution is likely to impede future work on the syntax.

I have a specific idea for a novel method of handling this flexibly, but more testing is required. I'm content to punt on this issue until that idea is fully operational and implemented in the wild. I'm optimistic that when that happens we'll be able to make everyone happy.

@wbond
Copy link
Member

wbond commented Dec 4, 2017

If the change involves breaking existing tests, I'm going to be against it at this point. This is one of those cases where I believe we need to keep backwards compatibility. It is amazing, but little changes like this greatly frustrate a certain profile of user, and I don't think we can rationalize breaking the aesthetics of their preferred editor config in a minor version release.

If we need to make things more consistent, I am fine with that, but not at the expense of breaking the color of such a prevalent syntax element. Unfortunately this either means keeping the scopes as is, or doing a survey of color schemes and seeing to what extent the scope changes would affect users of Package Control.

@FichteFoll
Copy link
Collaborator

FichteFoll commented Dec 6, 2017

By the way, you might be interested in joining the Sublime Text discord server, where pretty much all of the regular contributors hang out and where some controversial stuff like this could be discussed more organically.

https://discord.gg/D43Pecu

Edit: That doesn't mean that issues are worthless. Especially for RFCs, being able to bring a long thought into a long post is much appreciated and the chat is more frequently used for quick feedback of such.


PS: This issue is leet.

@deathaxe deathaxe added the RFC label Aug 20, 2021
@michaelblyons
Copy link
Collaborator

The 4xxx changes to core have opened a new possibility, wherein JavaScript with jQuery could be an extension of a core JavaScript syntax.

People who complain could be nudged to make the extended one their primary .js syntax. The tricky bit may be what to do about <script> tag embeds.

@deathaxe
Copy link
Collaborator

Script tags can't be handled in sane ways with current possibilities.

We would need something like VS Code's inject, which enables any third-party package to hook into a primary syntax definition to extend its capabilities. The difference is an end user never needs to select a special syntax, just keep working with the core syntax.

Situations like this or even Markdown would benefit much from it. Any syntax package could create an injection rule to add itself to the list of fenced code blocks.

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

No branches or pull requests

6 participants