Skip to content

Semantic Tokens: Distinguish (possibly-partially-applied) infix operators from normal functions/variables #3969

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

Closed
konn opened this issue Jan 16, 2024 · 16 comments
Assignees

Comments

@konn
Copy link
Collaborator

konn commented Jan 16, 2024

Is your enhancement request related to a problem? Please describe.

First of all, thank you @soulomoon for implementating fantastic support for semantic tokens! I've long been waiting for this feature 🎉

After the release of 2.6.0.0, I've just tested it and looks really good. I think it will become even better if one can tell (possibly partially-applied) infix operators from ordinary variable/functions semantically.

For example, the image below is what I apply One Monokai + Semantic Tokens to the code fragments containing several ($)s:

code-semantic

In above, functions being applied and ($)s are all coloured homogeneously. Without semantic colouring, OTOH, we got:

code-syntactic

I think the latter case gives more legible highlighting - binary operator shines between functions.

Describe the solution you'd like

Add something like TBinaryOperator constructor to HsSemanticTokenType, map its default to operator token, and also allowing users to map them to what they like (as we already did for haskell.plugin.semanticTokens.config.dataConstructorToken etc).

Describe alternatives you've considered

Perhaps, as we have already done to plain symbols, there can be TBinaryFunction and TBinaryVariable, as the function declaration can take its argument in infix-form.

@soulomoon
Copy link
Collaborator

soulomoon commented Jan 16, 2024

Thank you @konn for the ideas! I've added it to the tracklist #3931

@michaelpj
Copy link
Collaborator

We even have a good LSP token type for this: operator.

@michaelpj
Copy link
Collaborator

michaelpj commented Jan 16, 2024

Also, what about `isSubsetOf` vs +? Is the former also an operator or should it continue to be a function?

@konn
Copy link
Collaborator Author

konn commented Jan 17, 2024

We even have a good LSP token type for this: operator.

Indeed, as I already suggested in my solution section 😃

Also, what about `isSubsetOf` vs +? Is the former also an operator or should it continue to be a function?

Good catch. Both seem making sense, but I prefer treating it as a function provided that ` is not treated as a symbol name and coloured differently.

@soulomoon
Copy link
Collaborator

soulomoon commented Jan 17, 2024

I realized, there is also a possible overlap here. take +, it is infix operator and class method.

@michaelpj
Copy link
Collaborator

Yep. I think you already have support for returning multiple types and picking one.

I note that there is a client capability for supporting overlapping tokens. I guess in principle that means we could return two tokens for the same span? I don't know what that would do.

@soulomoon
Copy link
Collaborator

soulomoon commented Jan 17, 2024

I note that there is a client capability for supporting overlapping tokens

I suspect it is scarcely supported by clients.
e.g. Vscode decided not to support this. microsoft/vscode#147774

I am inclined to take precedency of class method over infix operator when it comes into conflict. What are you guys opinions on this?

@michaelpj
Copy link
Collaborator

I agree

@konn
Copy link
Collaborator Author

konn commented Jan 17, 2024

I note that there is a client capability for supporting overlapping tokens

I suspect it is scarcely supported by clients. e.g. Vscode decided not to support this. microsoft/vscode#147774

I am inclined to take precedency of class method over infix operator when it comes into conflict. What are you guys opinions on this?

I prefer it to be infix operator, but it is just a matter of taste. We might be able to make such precedence preference configurable, but that seems rather complex and non-ergonomic. After all, humans can tell the infix operators from plain class method as it consists of (non-letter) symbols. So, yes, treating it as class method seems making sense to me.

@soulomoon
Copy link
Collaborator

soulomoon commented Jan 17, 2024

  1. class method is not as common as function
  2. operator is needed since we want to differ them from function
  3. it leads to wether we want to know it is an operator or it is class method more.

I think operator(as class, function, class method, field, type operator) might be bertter off as modifier.

But default LSP does not have such modifier.

@michaelpj
Copy link
Collaborator

We have several clashes already, in fact:

  • function vs not
  • operator vs not
  • method vs not

Something can be all of a function, an operator, and a method!

On the configurability front, we could consider making it so that we don't take certain things into account. So e.g. you could turn off highlighting methods entirely, with the intention that you now get the next token assignation that makes sense.

Alternatively we could expose priority options for the different token types (as integers), and let people change them.

@soulomoon
Copy link
Collaborator

soulomoon commented Jan 17, 2024

Yes, it does make sense to offer more configurability.
The priority options seems to be a better choice, we can always map things we do not care as much to lower priority.
Either way, it makes more sense to use nested configurations now. #3944

@soulomoon
Copy link
Collaborator

#4030 should fix this now, give a try if it is working well? @konn

@konn
Copy link
Collaborator Author

konn commented Feb 4, 2024

@soulomoon Thanks! And, sorry for my late reply (I had been a bit busy for the past two weeks). I will give it a try.

@konn
Copy link
Collaborator Author

konn commented Feb 4, 2024

Looks good! Here is the example (I switched the colour theme from One Monokai to Material Theme Ocean, as the former doesn't work well with semantic tokens for the time being).

Without semantic tokens:

code-plain

... and with semantic tokens:

code-semantic

Now we can distinguish binary operators from functions! Looks really good 😄

I also tested `func`-style infixes:

Without semantic tokens:

code2-plain

... and with semantic tokens:

code2-semantic

The above example has both symbolic and `func`-style (backticked) operators.
Without semantic tokens, default syntax defs treat backticked operators, as a whole, as a kind of infix operators.
With semantic tokens, the quot function itself is highlighted as a function, and a backtick as a punctuation.

language haskell
standard token type Other
foreground #89DDFF
background #0F111A
contrast ratio 12.41
item value
textmate scopes punctuation.backtick.haskell, keyword.operator.function.infix.haskell, source.haskell
foreground punctuation { "foreground": "#89DDFF" }

I think the current treatment is good enough - after all, quot itself is a function and what makes it infix-style is ` and they are treated differently, indicating the details of how the things work.

@soulomoon
Copy link
Collaborator

soulomoon commented Feb 4, 2024

Thank you for the detailed feed back on this. @konn

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

When branches are created from issues, their pull requests are automatically linked.

3 participants