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

Implement semantic tokens plugin to support semantic highlighting(textDocument/semanticTokens/full) #3892

Merged
merged 81 commits into from
Jan 6, 2024

Conversation

soulomoon
Copy link
Collaborator

@soulomoon soulomoon commented Dec 6, 2023

Working out #1654
specification: specification

motivation

It enhance the original syntax highlighting by differing tokens semanticly.
image

What is implemented

Features:

  • Supported PluginMethodHandler

  • Supported semantic tokens type:

    • class and class method
    • type family name (data family)
    • data constructor name (not distinguishing record and normal data, and GADT)
    • type constructor name (GADT)
    • record field name
    • type synonym
    • pattern synonym
    • pattern bindings In favor of differing functions and none-functions from its type
    • value bindings In favor of differing functions and none-functions from its type
    • functions
    • none-function variables
    • imported name
  • Supported modifiers(planning):

    • [future] declaration (as in class declearations, type definition and type family)
    • [future] definition (as in class instance declaration, left hand side value binding, and type family instance)
    • [future] modification (as in rec field update)
  • Support unicode

Implementation details:

The focus of this implementation of semantic tokens is to support semantic highlight.
modifiers does not seems to provided as much use in pure language as in impure language.
I decided to ignore the modifier support for now, and schedule to the future.

Ps
I am using the following configuration to test the stuff out in vscode.

    "editor.semanticTokenColorCustomizations": {
        "enabled": true, // enable for all themes
        "rules": {
            "interface": {
                "foreground": "#daf049"
            }
            ,
            "type": {
                "foreground": "#f81eba"
            }
            ,
            "typeParameter": {
                "foreground": "#cd4ca0"
            }
            ,
            "property": {
                "foreground": "#f81e"
            },
            "method": {
                "foreground": "#1ef1f8"
            },
            "class": {
                "foreground": "#ffe100"
            }
        }
    },

@soulomoon soulomoon mentioned this pull request Dec 6, 2023
@soulomoon

This comment was marked as resolved.

@soulomoon soulomoon changed the title draft: Implement semantic tokens lsp plugin draft: Implement semantic tokens lsp plugin to support semantic highlighting Dec 11, 2023
@soulomoon soulomoon marked this pull request as ready for review December 14, 2023 13:32
@soulomoon
Copy link
Collaborator Author

I would be great if we could transfer some of the reasoning you've written in the PR into code comments. In particular, we've had a bit of discussion about:

  1. What token types to use for what kinds of Haskell entity
  2. What information we can and can't get from what sources

This seems likely to be useful to future people working on this plugin and will get lost if it's just in PR comments.

sure, I'll collect them some times later

@Bodigrim
Copy link
Contributor

but since text reading involved, the conversion taken place one by one is not efficient, maybe we can create a position mapping for the whole file at one. And cache that mapping to do the conversion instead.

To avoid performance degradation text-rope supports mixed indexing mode in Data.Text.Utf16.Rope.Mixed. It would be great if someone revived haskell/lsp#436 and switched lsp to use it.

@soulomoon
Copy link
Collaborator Author

soulomoon commented Dec 31, 2023

but since text reading involved, the conversion taken place one by one is not efficient, maybe we can create a position mapping for the whole file at one. And cache that mapping to do the conversion instead.

To avoid performance degradation text-rope supports mixed indexing mode in Data.Text.Utf16.Rope.Mixed. It would be great if someone revived haskell/lsp#436 and switched lsp to use it.

Sure, I can take a look at them in the near future to see if I could be helpful.

@michaelpj
Copy link
Collaborator

Could you make this disabled by default, like the stan plugin here? #3917

I think given how big and new this is it's sensible to do that for one release and get some testing.

Then I think this is close to being mergeable. I don't know if there's more tidying you want to do - maybe a bit more documentation/cleaning up commented out code etc.

And we should remember to make issues for the bits of future work you have in mind.

@soulomoon soulomoon requested a review from fendor as a code owner January 5, 2024 15:52
@soulomoon
Copy link
Collaborator Author

soulomoon commented Jan 5, 2024

Could you make this disabled by default, like the stan plugin here? #3917

I think given how big and new this is it's sensible to do that for one release and get some testing.

Then I think this is close to being mergeable. I don't know if there's more tidying you want to do - maybe a bit more documentation/cleaning up commented out code etc.

Yep, Plugin is off by default now and tidying is done.

And we should remember to make issues for the bits of future work you have in mind.

definitely, I will open issues to track them down.

@michaelpj michaelpj enabled auto-merge (squash) January 5, 2024 16:50
@soulomoon
Copy link
Collaborator Author

track list have been made in #3931

@soulomoon
Copy link
Collaborator Author

soulomoon commented Jan 5, 2024

it seems a [Required] test is failling accidentally. Maybe it need to be rerun @michaelpj

ghcide
  diagnostics
    typecheck-all-parents-of-interest: FAIL (0.36s)
      test\exe\DiagnosticTests.hs:493:

@michaelpj michaelpj merged commit 37925a0 into haskell:master Jan 6, 2024
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants