haskell-language-server icon indicating copy to clipboard operation
haskell-language-server copied to clipboard

Document symbols could include local definitions

Open michaelpj opened this issue 2 years ago • 8 comments

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

michaelpj avatar Jun 08 '23 11:06 michaelpj

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

wz1000 avatar Jun 08 '23 12:06 wz1000

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?

sgillespie avatar May 23 '24 01:05 sgillespie

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?

michaelpj avatar May 23 '24 09:05 michaelpj

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

sgillespie avatar May 24 '24 18:05 sgillespie

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

  • f
    • g
      • h

(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?

sgillespie avatar Jul 09 '24 02:07 sgillespie

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".

michaelpj avatar Jul 09 '24 16:07 michaelpj

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 avatar Jul 10 '24 02:07 sgillespie

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.

sgillespie avatar Sep 06 '24 13:09 sgillespie