search

haskell / haskell-language-server

Official haskell ide support via language server (LSP). Successor of ghcide & haskell-ide-engine.
Apache License 2.0
2.71k stars 368 forks source link

support module name and split out module name in qualified name #3957

Closed soulomoon closed 9 months ago

soulomoon commented 9 months ago

Mainly for quolified names. Semantic highlighting looks worse than syntax highlighting for quolified name.

  1. A Syntax highlighting easily support splitting the module part and the name part of a quolified name.
  2. Semantic token is not identifing them, since the range of Name for quolified name include the module part and the name part as a whole.

Two steps to solve the problems.

  1. First add suppport for module names in the import section, it is easy since there is module name as identifier in hieAst #3958
  2. split out the module part and name part, handles (Prelude.+), `Data.List.elem` #3958
soulomoon commented 9 months ago

Two design decisions of step 2 need to be nail down.

  1. Step 2 requires peeking into the source code, which somewhat breaks the abstraction of LSP and reach into the LSP's package dependency Text.Rope, I am not quite sure if it is a good idea to implement Rope related function here. Excepcially when https://github.com/haskell/lsp/pull/542(unrelease) get into release and HLS picks it up, Rope related function implemented in HLS would surely break. Maybe it would be nice to have API such as getTextByCodePointRangeFromVfs in LSP(Like the one already there rangeLinesFromVfs) ? And I was wondering when would the next LSP version get release @michaelpj
  2. for (Prelude.+), it is original highlight (Prelude.+) as a whole, now we need to split them.
    1. break into (Prelude. and +).
    2. drop the () and break into Prelude. and +
michaelpj commented 9 months ago

Step 2 requires peeking into the source code

Hmm. Why do we need to go into the source code? Don't we have the OccName which I think includes the full name text? Then we could just look at that, count how many characters until the last ., and split the Range at that point? Will need a bit of care about what "character" means, but I don't think we necessarily need the whole file source.

for (Prelude.+), it is original highlight (Prelude.+) as a whole, now we need to split them.

Wait, we currently highlight the parens? That's surprising! Is that because GHC gives that as part of the span?

soulomoon commented 9 months ago

Wait, we currently highlight the parens? That's surprising! Is that because GHC gives that as part of the span?

Yes, HieAst gives span of (Prelude.+), and OccName gives only + in its Fastring. GHC handling of qualified name is not ideal for us. This is the same reason we need to peek into the source code, since we don't have the full text (Prelude.+) anywhere in Name nor OccName(Or we should say, not anywhere in the HieAst).

michaelpj commented 9 months ago

That seems... surprising to me. The span includes all the other text, but the OccName doesn't? how odd. Is that expected @wz1000 ?

soulomoon commented 9 months ago

You can see that from the renamed source as well. If my memory does not fail me, I was once saw an GHC issue about this handling of qualified name long time ago, but could not find it since I forget about the keyword to search for it. Turn out it is the same for (operator), `function`