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

MEGA-ISSUE: Syntax highlighting in language-typescript #874

Open
savetheclocktower opened this issue Jan 19, 2024 · 8 comments
Open

MEGA-ISSUE: Syntax highlighting in language-typescript #874

savetheclocktower opened this issue Jan 19, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@savetheclocktower
Copy link
Contributor

savetheclocktower commented Jan 19, 2024

IMPORTANT: Some issues have already been fixed!

If you’re still on the regular 1.113 release, you might be suffering from a problem that has already been fixed. Many fixes landed on master in #859. You are encouraged to download the latest rolling release — both (a) to see whether what you’re reporting has been fixed, and (b) so that you can enjoy the fixes that have been reported since the 1.113 release.


This will serve as a catch-all issue for any problems you may encounter with syntax highlighting in TypeScript or TypeScript (React) files (language-typescript). If you have problems with the new syntax highlighting (or folds, or indents) that are specific to ts or tsx files, keep reading.

Something isn't highlighting correctly!

If there are any highlighting regressions since 1.113:

First, please scroll down and see if someone else has reported the issue. If not, please comment with

  • A small amount of sample code to reproduce the issue
  • Screenshots (before and after would be ideal, but just the “after” is OK for obvious things

I want to go back to the old highlighting!

You can easily opt out of the new Tree-sitter highlighting for any language, but first please:

  • subscribe to this issue so you'll know when the problem is fixed
  • remove the config change when the next release comes out so that you can enjoy the new grammar once again

To revert to the old Tree-sitter grammar for this language only, add the following to your config.cson:

".source.ts, .source.tsx":
  core:
    useLegacyTreeSitter: true

To revert to the TextMate-style grammar for this language only, add the following to your config.cson:

".source.ts, .source.tsx":
  core:
    useTreeSitterParsers: false
@savetheclocktower savetheclocktower added the bug Something isn't working label Jan 19, 2024
@savetheclocktower
Copy link
Contributor Author

savetheclocktower commented Jan 19, 2024

@jakubhruby has reported that objects of the form foo.bar.baz have lost their highlighting. This is a known issue and will be addressed. This has been addressed; see below.

example

@savetheclocktower
Copy link
Contributor Author

Regarding the issue above:

I've just added some scopes into TypeScript files that were previously only present in JavaScript files. This ensures that they'll be scoping long object chains (like the ones in the screenshot above) identically.

Those scope names have changed from the ones assigned in the legacy Tree-sitter grammar. Intermediate segments of the chain are scoped as support.other.property rather than variable.other.object.property, and the last segment of the chain is only scoped as variable when it's on the left-hand side of an assignment.

But I don't want that change to feel disruptive, so I've just made a corresponding change in the One Dark and One Light syntax themes to ensure that any scope name with a property segment is highlighted like a variable. I think this has a low chance of causing unwanted side effects while keeping everyone happy.

These changes are now part of #859 and will go out no later than mid-February in Pulsar 1.114, and perhaps sooner if we can manage it.

@jakubhruby
Copy link

@jakubhruby has reported that objects of the form foo.bar.baz have lost their highlighting. This is a known issue and will be addressed. This has been addressed; see below.

example

The same part of code while Using legacy tree-sitter implementation set to true:
image

@jakubhruby
Copy link

It's not only the chained props highlighting, but also called functions (in top of the screenshot) and object key-value pairs (e.g. {deep: true} in the screenshot) and several other problems. I can provide more examples if needed.

@savetheclocktower
Copy link
Contributor Author

It's not only the chained props highlighting, but also called functions (in top of the screenshot) and object key-value pairs (e.g. {deep: true} in the screenshot) and several other problems. I can provide more examples if needed.

Whatever you can provide I'd be happy to look over. I want to make sure we get this right.

As for the things you mentioned:

  • The color of ordinary functions like the watch in your screenshot has been addressed in this commit.

  • The keys in object literals have been scoped a bunch of different ways in different JavaScript/TypeScript grammars. The old Tree-sitter grammars are where they first started getting scoped as variable; in the grammars that shipped with TextMate, they were entity.name.type.attribute-name. In the Babel ES6 grammar they were constant.other.object.key. In VSCode (just for fun) they're scoped as meta.object-literal.key.js.

    Those properties can plausibly be variables on usage, but on definition they're more like attribute names. If enough people are bothered by the color change, I'll change the built-in themes so that object keys are colored identically to other variables, but for semantics I had to pick something that wasn't in the variable namespace. At any rate, if you want those keys to be red, I can give you the CSS override for your user stylesheet.

@savetheclocktower
Copy link
Contributor Author

I notice that Object is colored… interestingly in your second screenshot.

Legacy Tree-sitter scopes the Foo in Foo.bar as support.typesupport makes sense, but type does not. It scopes the foo in foo.bar as nothing at all. Because Object has a capital first letter, it gets the support.type treatment.

Instead, I decided that the Object in Object.assign would be support.builtin (because it's a recognized global object in JavaScript/TypeScript), but both Foo and foo as described above would be support.other.object (because both are ordinary objects, and scoping them differently based on casing is drawing a distinction that JavaScript itself doesn't draw).

This might be another situation where the theme changes to agree with folks‘ expectations, or it might be one where I change a scope name in the spirit of compromise. The goal is that, no matter what we land on, there's enough detail in the scope names so that people can override these choices however they want. Some of the old scopes were terse enough that you couldn't customize how one thing looked without inadvertently changing something else.

@jeffplays2005
Copy link

jeffplays2005 commented Apr 5, 2024

  • Noticed a syntax highlighting issue here.
Screenshot 2024-04-05 at 1 06 22 PM
  • Formatting string seems to also have some missing syntax colors.
Screenshot 2024-04-05 at 1 11 34 PM Expected: Screenshot 2024-04-05 at 1 11 44 PM

@savetheclocktower
Copy link
Contributor Author

@jeffplays2005, those were both very easy to address. They're part of #968 and will ship in 1.116 — probably in about 11 days.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants