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

Support highlight for constructor call #2124

Closed
OzelotVanilla opened this issue Sep 20, 2021 · 9 comments · Fixed by #2146
Closed

Support highlight for constructor call #2124

OzelotVanilla opened this issue Sep 20, 2021 · 9 comments · Fixed by #2146

Comments

@OzelotVanilla
Copy link

The highlighter for Java cannot differ constructor from function call. Hope the syntax file can be edited so we can add colour to constructor call.

In this picture, the left File is highlighted as class, however the constructor call is highlighted as function.

image

@fbricon
Copy link
Collaborator

fbricon commented Sep 20, 2021

that's a question for @0dinD

@OzelotVanilla
Copy link
Author

that's a question for @0dinD

Thank you.

@0dinD
Copy link
Contributor

0dinD commented Sep 20, 2021

I agree that this would be useful, and it was almost implemented in eclipse-jdtls/eclipse.jdt.ls#1514, but after some discussion with @Eskibear I decided not to add the feature for now to avoid the complexity of backwards-compatibility for old themes.

The problem is that changing the token type of constructors might be a breaking change for themes that desire the same color for constructors and methods. One question is whether or not this really matters, as scope mapping could be added to fix TextMate themes, and explicit semantic token rules might not be as widely adopted. A second question is, given the answer to the first question, what is the optimal way to represent constructors using semantic tokens and modifiers to allow flexibility and/or backwards-compatibility for theme authors?

Alternative 1

Use the type of the constructor (class, record, enum) as the token type, and add the constructor modifier. Given the code in your screenshot above, the first File would be a class and the second would be a class.constructor. Styling all constructors at once could be done via the *.constructor rule.

Pros:

  • Is flexible, makes sense semantically
  • Only introduces 1 new modifier

Cons:

  • Not fully backwards-compatible, since theme authors need to manually add the *.constructor rule if they're using semantic token rules and want constructors to keep the same color as methods

Alternative 2

Introduce a new constructor token type for constructors, as a subtype of method. Given the code in your screenshot above, the first File would be a class and the second would be a constructor.

Pros:

  • Introduces no new modifiers
  • Is fully backwards-compatible (as long as method is declared the superType of constructor)

Cons:

  • Is not flexible (you cannot have different styles for class constructors vs enum constructors etc)

Alternative 3

Introduce a new token type for each type of constructor (classConstructor, recordConstructor, enumConstructor). Given the code in your screenshot above, the first File would be a class and the second would be a classConstructor.

Pros:

  • Introduces no new modifiers
  • Is fully backwards-compatible (as long as method is declared the superType of each new token type)

Cons:

  • Feels a bit like "token duplication" (as in, "code duplication")
    • This may also be annoying for theme authors, as they'll also need duplicated rules to style e.g. class types the same as class constructors.

Alternative 4

Introduce new token modifiers for the types (class, record, enum) as well as constructor. Given the code in your screenshot above the first File would be a class and the second would be a method.class.constructor.

Pros:

  • Is flexible, makes sense semantically
  • Opens up even more themeing options, such as distinguishing between method.class and method.enum
  • Is fully backwards-compatible

Cons:

  • Introduces many new token modifiers
    • This may or may not be a problem, but consider that the maximum number of modifiers we can encode is 32 since we are using 32-bit integers. So utilizing existing token types as done in alternative 1 is probably preferable.
  • Similar to alternative 3, feels a bit like "token duplication" and will have the same effect for theme authors.

In conclusion: the feature itself is easily implemented, but we need to consider the semantics and backwards-compatibility. @OzelotVanilla @Eskibear What do you think? It's also totally possible I've missed something or there's a better alternative.

@Eskibear
Copy link
Contributor

I vote for Alternative 1, for it looks more like a systematic approach.

@OzelotVanilla
Copy link
Author

@0dinD I also think Alternative 1 is good. By the way, is there a detailed tutorial for compose tmLanguage file? I have read the VS Code's docs, but I still have many question. Could you tell me how you learn writing tmLanguage file, if you want to tell me.

@0dinD
Copy link
Contributor

0dinD commented Sep 22, 2021

I agree, just wanted to make sure we were on the same page about compatibility. I'll submit a PR as soon as I have time, hopefully we can get this into the next update.

By the way, is there a detailed tutorial for compose tmLanguage file? I have read the VS Code's docs, but I still have many question. Could you tell me how you learn writing tmLanguage file, if you want to tell me.

Just so there's no misunderstanding, you mean writing a TextMate grammar for tokenizing a language, not theming right (difference explained here)?

I don't have much experience writing TextMate grammars myself, but the main thing you'll need to learn if you haven't already are Regular Expressions. Also, I'd probably take a look at existing grammars to learn (once you know the basics), as there don't seem to be many guides available on the topic. I think this mostly has to do with TextMate being an old standard that most editors are moving away from for various reasons. One of the main limitations is that since TextMate grammars are only based on Regular Expressions, they cannot provide context from the project or other files, the tokenizer works with each file separately.

That's why VS Code has added a new API called Semantic Highlighting where language extensions (like this one) can be used to provide tokenization based on their rich knowledge of the language and project files. This can be implemented in any way you prefer, as it's just a generic API: VS Code asks the Java extension to tokenize the file Foo.java and the extension then replies with a result. In the case of this extension, the request is handed to the Eclipse JDT Language Server, but other libraries are used for other languages.

You can already see the difference in your screenshot: the TextMate grammar tokenized File into the scope entity.name.function.java (just a generic function), whereas the Semantic Highlighting provided by this extension also gave the public modifier (which the TextMate grammar had no way of knowing about). And in the future we'll also be able to say it's a class constructor, as discussed in this issue.

@OzelotVanilla
Copy link
Author

@0dinD Thank you very much that you want to tell me how to tokenize and the semantic highlighting. Actually I want to make highlight of a programming language (but now there is only me on this work). I really like the idea of Semantic Highlighting. I think I should learn the Semantic API. Thanks again. Hope the "constructor idea" can come true.

By the way, RedHat formatting file need the abs path of the file, and file:/// prefix. If a path contains space or CJK characters, I think the user may feel confusing that their URL cannot be used (for me, I change my space to %20, and solve the bug. However, if the path is invalid, I think there is no error log output in vscode "log output" page (in the terminal)).

@0dinD
Copy link
Contributor

0dinD commented Sep 24, 2021

No problem!

By the way, RedHat formatting file need the abs path of the file, and file:/// prefix. If a path contains space or CJK characters, I think the user may feel confusing that their URL cannot be used (for me, I change my space to %20, and solve the bug. However, if the path is invalid, I think there is no error log output in vscode "log output" page (in the terminal)).

Well, the setting does say it takes a URL, and spaces are not valid characters in URLs (hence the %20 encoding). Although I agree that this may be confusing for new users, and it could be handled by the extension instead (you'd have to open a new issue for that).

@fbricon
Copy link
Collaborator

fbricon commented Oct 12, 2021

See constructor customization:
Screenshot 2021-10-12 at 09 32 51

@fbricon fbricon added this to the Mid October 2021 milestone Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants