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

Semantic tokens features trackList #3931

Open
18 of 25 tasks
soulomoon opened this issue Jan 5, 2024 · 24 comments
Open
18 of 25 tasks

Semantic tokens features trackList #3931

soulomoon opened this issue Jan 5, 2024 · 24 comments
Assignees

Comments

@soulomoon
Copy link
Collaborator

soulomoon commented Jan 5, 2024

Semantic highlighting would give us a great experience on reading the code. It's been a shame that haskell does not have it for all these times. But now we are going to make it happen for haskell in HLS. (Thank @michaelpj for his pull request that support it in lsp package), first version of semantic tokens have been made in this pull request. (Thank @michaelpj @wz1000 for all the help and reviews. )
But since the amount of works it need , not everything in the LSP semantic tokens specification is supported yet, we would do them step by step.
Here is the track list of features.

@Bodigrim
Copy link
Contributor

Bodigrim commented Jan 6, 2024

Thanks for working on this feature! I gave it a quick try in SublimeText on https://github.com/Bodigrim/cabal-add/blob/a83c827e45eeaed722d63f08a4c1bfd183770e97/src/Distribution/Client/Add.hs#L189:
image
Is it something off with my configuration? Why String is in blue, but ByteString is white?

@michaelpj
Copy link
Collaborator

The current implementation highlights type synonyms differently to datatypes. So String and FilePath are different because they are type synonyms. Maybe this is confusing! (The fact that you asked suggests that it is...)

It's a bit funny that you get the same colour for parseCabalFile and MonadError also. Generally it seems a bit hit-and-miss whether any given colour scheme actually distinguishes everything that's in the spec...

@Bodigrim
Copy link
Contributor

Bodigrim commented Jan 6, 2024

Maybe this is confusing! (The fact that you asked suggests that it is...)

Yeah, that's very confusing, especially given how ubiquitous String and FilePath are. I think it would be better not to distinguish.

@soulomoon
Copy link
Collaborator Author

soulomoon commented Jan 7, 2024

Thank you for testing it out. @Bodigrim
We may need a bit of settings for some color schemes to get the colors right. The mapping from hs semantic token type to the standard semantic token type in lsp is here.

https://github.com/soulomoon/haskell-language-server/blob/master/plugins/hls-semantic-tokens-plugin/src/Ide/Plugin/SemanticTokens/Mappings.hs

say if we want the color scheme to not distinguish between a normal datatype with a type synonyms.
we can always set the Type( type synonym) and Enum(a normal datatype) to have the same color.

=====================================================
To be precise, we are not actually doing the semantic highlighting, we are giving semantic types to the tokens that enables the semantic highlighting feature for haskell in the editors. Maybe it worth a note some where to help user to configure this if the color scheme does not look good. Expecially with the fact that there is a layer of mappings between hs semantic to lsp semantic. And the color scheme is various.

=====================================================

It's a bit funny that you get the same colour for parseCabalFile and MonadError also. Generally it seems a bit hit-and-miss whether any given colour scheme actually distinguishes everything that's in the spec...

Also I was thinking🤔, maybe we can also make the toLspTokenType configurable to provide more flexibility on this. In this way, we can provide as detailed as much for the semantic token type, but also enable the user to configure their flavor if the default mapping does not suit their need.

@soulomoon
Copy link
Collaborator Author

soulomoon commented Jan 7, 2024

Maybe this is confusing! (The fact that you asked suggests that it is...)

Yeah, that's very confusing, especially given how ubiquitous String and FilePath are. I think it would be better not to distinguish

I do not have a SublimeText subscription to test things out, but most color scheme in vscode do not distinguish them by default.
Here is the the default mappings look like in vscode with the scheme one dark pro.

Screenshot 2024-01-07 at 16 45 34

But type synonym might be constraint too. This is one of the reason why we don't classify type synonym to have the same token type as normal data type.

@michaelpj
Copy link
Collaborator

(Possibly we should split this off into another issue)

Worth pointing out that type synonyms are highlighted as "type" and in this case the difference is because datatypes get a special highlighting.

maybe we can also make the toLspTokenType configurable to provide more flexibility on this

Generally we try pretty hard to avoid configuration. It's usually much better if we can find something that's universally acceptable.

I guess I could maybe be convinced to have settings for each thing where we have a choice of a specific or generic token type, e.g. "special highlighting for type synonyms: y/n", and if it's off then we use the generic one (e.g. "just a type"). We could let people pick exactly the LSP semantic token type for each thing but that's probably overkill.

We do already have a configuration switch here: it's whether or not you have semantic tokens on or off. If you turn semantic tokens on, you should probably expect it to make some fine distinctions that aren't obvious to you at first sight. So a different question is "how do we make this discoverable?". Perhaps in this instance having the options would serve a double function: it tells you what kinds of things we distinguish, and then you can fiddle with it to see what it does.

Having written all that I guess I've convinced myself that we should have options. Especially since a lot of the things semantic tokens lets you do are quite opinionated and specific, which some people will inevitably dislike (e.g. I bet people will complain about the different highlighting of things of function type).

@michaelpj
Copy link
Collaborator

I split off another issue for discussing the configuration issue: #3937

@michaelpj
Copy link
Collaborator

@soulomoon a couple of minor things that we still need to do:

@soulomoon
Copy link
Collaborator Author

soulomoon commented Jan 9, 2024

@michaelpj thanks for sort it out. do it in #3938

@michaelpj
Copy link
Collaborator

A possible concern: there is a multilineTokenSupport client capability. We probably need to respect that as well as overlappingTokenSupport. I think the current implementation might return multi-line tokens if something goes over several lines.

I think we could maybe fix this universally in lsp: the functions that compute semantic tokens could also take the ClientCapabilities, and then do something like:

  • If multilineTokenSupport is false, then split any multi-line tokens into one token per line
  • if overlappingTokenSupport is false, then split any overlapped tokens into [<part before overlap>, <part after overlap>]

@michaelpj
Copy link
Collaborator

I actaully don't know if we'll emit multi-line tokens today already, do you know @soulomoon ? I guess we probably don't, since all the stuff we recognize today can't be split over multiple lines anyway.

@soulomoon
Copy link
Collaborator Author

soulomoon commented Jan 18, 2024

I actaully don't know if we'll emit multi-line tokens today already, do you know @soulomoon ? I guess we probably don't, since all the stuff we recognize today can't be split over multiple lines anyway.

no we do not support them yet.

@michaelpj
Copy link
Collaborator

I made an issue for doing something helpful in lsp: haskell/lsp#545

@soulomoon soulomoon self-assigned this Feb 3, 2024
@MangoIV
Copy link

MangoIV commented May 18, 2024

pretty cool! do you think it would be somehow possible to highlight parameters to a function? I could not figure out how that would work, i.e. that in

f = \x -> x y z

x gets a highlight group and that reoccurs in the body where x is in scope.

@MangoIV
Copy link

MangoIV commented May 18, 2024

I've really enjoyed the possibility of distinguishing class and type/ enum.
image

something that I'm wondering is why you chose to implement class and not interface, when I'm using the same colour schemes across languages, I think semantically it would make more sense if haskell classes would not be coloured like java classes but rather like java interfaces or abstract classes or whatever.

@michaelpj
Copy link
Collaborator

pretty cool! do you think it would be somehow possible to highlight parameters to a function?

There is a paramter token type. I'm unsure if that's supposed to be used for the declaration of a parameter or also its uses, since it means we can't mark it as a plan "variable". But I think it would be reasonable to use it for such things. We also have a bit of confusion because we use the function token type for variables of function type, so then we start to have difficulties: what token type do we give for a variable that is both a parameter and a function?

something that I'm wondering is why you chose to implement class and not interface

I think either is plausible, I think @soulomoon just picked one. I guess on reflection interface is maybe a bit more accurate.

@soulomoon
Copy link
Collaborator Author

soulomoon commented May 18, 2024

Since we do have class and not to match class with class is rather confusing.

We won't have a perfect match on Haskell token vs default token. So we actually make it configurable. So you can pick the matches that suit your color scheme. @MangoIV

@MangoIV
Copy link

MangoIV commented May 18, 2024

surely, yes, but I wonder what would happen if someone would program in both haskell and an object oriented language :D (I know even then you can configure it but perhaps we should optimize for the stock experience :P)

@soulomoon
Copy link
Collaborator Author

soulomoon commented May 18, 2024

Yes, since haskell do have class term, it is a trade-off between prioritizing haskell term vs prioritizing object oriented language term. As a language server for haskell, I think the choice would be obvious? Although for the ones without default token, we are compromising by projecting them.

For the paramter, I suppose it might a good idea to have it. But, yes it is not easy to classify them in hieAst, so it is not implemented at the first place.

@rumkeller
Copy link

@michaelpj

There is a paramter token type. I'm unsure if that's supposed to be used for the declaration of a parameter or also its uses

This seems to suggest that it's meant for both.

what token type do we give for a variable that is both a parameter and a function?

Here is an example of how the text editor deals with multiple types for the same token. So perhaps it's o.k. to emit both variable and parameter types for the same token?

@soulomoon
Copy link
Collaborator Author

soulomoon commented Sep 3, 2024

So perhaps it's o.k. to emit both variable and parameter types for the same token

@rumkeller AFAIK, VSCode would consider them as overlapping token which they do not support.

@rumkeller
Copy link

You're right -- it seems there can indeed be only one token type (though several modifiers). Apologies for the confusion.

So one has to choose between variable, function, and parameter. FWIW, here is what clangd does:

  • Parameters are always marked as such, even if they are function pointers.
  • It adds functionScope and globalScope modifiers. functionScope is for both parameters and local variables.

@michaelpj
Copy link
Collaborator

It adds functionScope and globalScope modifiers. functionScope is for both parameters and local variables.

These are non-specified modifiers, right? Do any clients check for them? (Generally it's a bit sad that we don't have an easy way to agree on extensions to the list of modifiers...)

@rumkeller
Copy link

These are non-specified modifiers, right?

Indeed they're not mentioned in the spec.

Do any clients check for them?

Neovim exposes the modifiers verbatim to the color scheme -- quite easy to adjust, though the default scheme appears to not be aware of them.

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

No branches or pull requests

5 participants