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

Add semantic highlighting #869

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Add semantic highlighting #869

wants to merge 9 commits into from

Conversation

pfoerster
Copy link
Member

@pfoerster pfoerster commented Mar 18, 2023

This PR adds basic support for semantic highlighting (full and range).

Token types:

  • Labels:
    • genericLabel
    • sectionLabel
    • floatLabel
    • theoremLabel
    • equationLabel
    • enumItemLabel
  • Citations:
    • genericCitation
    • articleCitation
    • bookCitation
    • collectionCitation
    • partCitation
    • thesisCitation
  • mathDelimiter

Token modifiers:

  • undefined
  • unused
  • deprecated

@clason

This comment was marked as resolved.

@pfoerster
Copy link
Member Author

@clason I think the client needs to be configured as well so it knows about the token types and the modifiers.

In VSCode, I had to do the following in the extension:

 "semanticTokenTypes": [
      {
        "id": "label",
        "superType": "type",
        "description": "A LaTeX label."
      }
    ],
    "semanticTokenModifiers": [
      {
        "id": "undefined",
        "description": "Annotates a symbol that is not defined"
      },
      {
        "id": "unused",
        "description": "Annotates a symbol that is not referenced"
      }
    ]

Then, I can set the colors separately using editor.semanticTokenColorCustomizations or let the theme handle if it does support the types/modifiers.

can I test this with standard names from the LSP spec, just to make sure?

Sure, if you edit the legend method and swap out the identifiers, it could maybe work out of the box.

@clason

This comment was marked as resolved.

@pfoerster
Copy link
Member Author

@clason

Hmm, I thought the server and client were supposed to negotiate the supported labels themselves. I'll see if I can add them manually to the client config somehow.

Yeah, it looks the client needs to explicitly support the token types. I think in this case, it is better to stick with the pre-defined ones. This is what VSCode sends for example:

"semanticTokens":{"dynamicRegistration":true,"tokenTypes":["namespace","type","class","enum","interface","struct","typeParameter","parameter","variable","property","enumMember","event","function","method","macro","keyword","modifier","comment","string","number","regexp","operator","decorator"],"tokenModifiers":["declaration","definition","readonly","static","deprecated","abstract","async","modification","documentation","defaultLibrary]"

@clason

This comment was marked as resolved.

@pfoerster
Copy link
Member Author

@clason Ah, it looks like Neovim does not support the range request. I can add the full one, too.

@clason
Copy link
Contributor

clason commented Mar 18, 2023

Yep, works like a charm now, including custom labels! Thanks!

@clason
Copy link
Contributor

clason commented Mar 18, 2023

I think inline $ is still fine in LaTeX (equivalent to \() , though; it's only the display $$ that should be avoided (not equivalent to \[) ;)

@clason
Copy link
Contributor

clason commented Mar 28, 2023

Just to be clear: the genericFoo tokens cover all labels/citations, so if someone does not want to distinguish all types, they can just define one token? (For example, I could see myself adding bold to bookCitation but leaving everything else generic.)

@clason
Copy link
Contributor

clason commented Mar 29, 2023

Maybe the type of the label/citation would make more sense as a modifier?

@pfoerster
Copy link
Member Author

Just to be clear: the genericFoo tokens cover all labels/citations, so if someone does not want to distinguish all types, they can just define one token?

Not (yet) with the current implementation but I think we need a fallback for new tokens, too. Not all clients will support them right at the start.

Maybe the type of the label/citation would make more sense as a modifier?

I thought about this before, but due to how modifiers are encoded via LSP (bit sets) we cannot use more than 32 or 64 depending on the client so we have little room in the future for expansion.

Maybe, the correct way would be to invert the token type in modifiers in this case. Define a book token type and let @book use the definition/declaration modifier and \cite{...} use the reference.

@clason
Copy link
Contributor

clason commented Mar 29, 2023

Hmm, in this case maybe the granularity of the token types is not worth it (I have a hard time thinking how I would distinguish all of these types visually...)

So my recommendation would be to drop this, actually, at least for now, and just stick to label/citation as types to attach modifiers to?

(On the other hand, Neovim allows me to just assign colors to modifiers themselves, so I wouldn't need to worry about the types for that.)

@clason
Copy link
Contributor

clason commented Apr 11, 2023

by the way, it was brought to my attention that unused and deprecated are actually already diagnostic tags in the LSP specification: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnostic

But as you already have the types, keeping these as token modifiers still makes sense?

@pfoerster
Copy link
Member Author

Hmm, in this case maybe the granularity of the token types is not worth it (I have a hard time thinking how I would distinguish all of these types visually...)

Yeah, this might be too much.

by the way, it was brought to my attention that unused and deprecated are actually already diagnostic tags in the LSP specification: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnostic
But as you already have the types, keeping these as token modifiers still makes sense?

I think it makes more sense to report unused labels and citations as diagnostic hints with the corresponding tags instead so that we do not need to duplicate the logic in the highlighting.

@clason
Copy link
Contributor

clason commented Apr 11, 2023

(there's still the undefined references, but those are then also probably better handled as diagnostics)

Guess my bright ideas for semantic tokens just collapsed....

@pfoerster
Copy link
Member Author

Guess my bright ideas for semantic tokens just collapsed....

Well, there is still the possbility to highlight commands depending on the source of the definition:

  • defined by the LaTeX kernel
  • defined by a package or documentclass
  • defined by the current project

@clason
Copy link
Contributor

clason commented Jun 17, 2023

Small ping; just wanted to ask whether you have any plans of adding the unnecessary and deprecated DiagnosticTags?

I can add an issue about it if it helps.

@pfoerster
Copy link
Member Author

@clason I am currently working on it (I did a small refactoring of the diagnostics module before ^^)

@pfoerster
Copy link
Member Author

@clason Can you try out #899, please? It implements additional diagnostics :)

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.

2 participants