haskell / haskell-language-server

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

Document symbols could include local definitions #3628

Open michaelpj opened 1 year ago

michaelpj commented 1 year ago

At the moment we include all the "top-level" stuff. But we could include local definitions as well. The thing that would be awkward is reflecting nested scopes, but often they are also associated with a symbol.

It might also be rather noisy, but arguably that's a client problem? e.g. things should be shown collapsed by default, if you open up a function, sure you get lots of stuff.

Examples to think about:

f x = g x
  where 
    g = ...

f x = 
  let g = ..
  in g x

f x =
   -- awkward nesting and shadowing
   let g = let g = ... in g
   in g x

f x = do
    -- nest g inside res? what if we discarded the result?
   res <- let g = ... in g x
   pure res
wz1000 commented 1 year ago

We have an interval map in Development.IDE.Spans.LocalBindings that could be used to implement this

sgillespie commented 5 months ago

Hi, I've been hacking at this, and here's an example of how it looks in emacs with eglot:

ss-20240522-213934

The markup itself looks like this:

* * *
Local bindings:

```haskell
hf :: HieASTs a
rm :: RefMap a
<-- Snip -->
prettyNames :: [Text]



How is this?
michaelpj commented 5 months ago

I'm a little confused about where you're adding this? I thought the idea was to put it into the document symbols, so it would show up in the outline, or similar?

sgillespie commented 5 months ago

Thanks for responding, I completely misunderstood the description. Will attempt to add it to textDocument/documentSymbol

sgillespie commented 4 months ago

After some discussion on Matrix, it sounds like what we really want is a something like this:

(assuming f, g, and h are nested local functions). The IntervalMap in Development.IDE.Spans.LocalBindings does not currently seem to give us this. Instead we end up with a flat list of names [f, g, h].

This does not seem to be so straightforward. What am I missing?

michaelpj commented 4 months ago

This does not seem to be so straightforward. What am I missing?

We might just need to change it! There's no guarantee the existing code does what we want. I guess one question is whether we would be okay with the non-nested version or whether that's going to be more confusing than not having local bindings...

Instead we end up with a flat list of names [f, g, h].

This is a little surprising to me, since AFAICT completions seem to work properly, i.e. in this program

f = goto 
  where
     goto = hello
      where
        hello = 1

I get "goto" in the completions at the top level but not "hello".

sgillespie commented 4 months ago

I get "goto" in the completions at the top level but not "hello".

This is because when we use the cursor position, we search upwards (getLocalScope), which gives us all names in scope at that point. To get all the local names in a top-level function, I believe we need to search the opposite direction (getFuzzyScope). However, AFAICT we're not getting this in a convenient form.

We might just need to change it! There's no guarantee the existing code does what we want. I guess one question is whether we would be okay with the non-nested version or whether that's going to be more confusing than not having local bindings...

This is certainly a possibility. We are currently using the IntervalMap from FingerTree which (to me) looks difficult to extend. We could roll our own that makes it easier to search subintervals.

sgillespie commented 2 months ago

We might just need to change it! There's no guarantee the existing code does what we want. I guess one question is whether we would be okay with the non-nested version or whether that's going to be more confusing than not having local bindings...

I asked about this on the fingertree issue tracker, but so far I haven't received a response.