feat(lsp): Implement document symbol outline#2298
Conversation
5d42b53 to
b014a7f
Compare
chore: Correct program outline loc feat(lsp): Implement document symbol outline
b014a7f to
5855b09
Compare
| List.filter_map( | ||
| (bind: Typedtree.value_binding) => { | ||
| switch (bind.vb_pat.pat_desc) { | ||
| | TPatVar(ident, _) => |
There was a problem hiding this comment.
Shouldn't we at least have TPatConstraint? I might argue that we'd want to process every TPatVar that appears in a toplevel binding.
There was a problem hiding this comment.
Doesn't TPatConstraint only appear under pat_extra?.
As per the others I would generally agree though I was thinking of leaving these for a small follow pr:
- cases like
let [x,y] = [() => void, () => void]in order to determine thatxandyshould have the kinds of functions I would have to start inspecting and breaking down theexp_typewhich seems a little complicated. Alternatively we could just say destructured items are of kindVariable. - I am not exactly sure what the range and selection ranges should be for destructured items.
There was a problem hiding this comment.
TPatConstraint is just when the user writes a type annotation. I don't think looking at the type is too bad, really. If it's an arrow then you know it's a function, and it can be a variable otherwise. As for range, for the big one I'd just do the whole binding, and for the small one I'd do just the name.
There was a problem hiding this comment.
TPatConstraint is under pat_extra though which means we don't need to look at right?
As per the other patterns what I really meant was two fold, patterns are recursive so I need to go down both the pattern and the type to figure out whats what i.e let [x,y] = [() => void, () => void] I need to start extracting the a from the list<a>, this isn't insanely difficult but it felt like a lot of complexity for the initial feature, this gets more complex when you consider you can have patterns like let [(x, y), (a, b)] = ... and other nested patterns, I am happy todo it though, I also don't know how well those selection ranges and ranges would work given vscodes symbol headers
For reference reasons lsp doesn't handle the nested patterns
I'm happy to add it if we really want it though.
There was a problem hiding this comment.
I think it's fine to start here.
ospencer
left a comment
There was a problem hiding this comment.
This looks really clean overall.
ospencer
left a comment
There was a problem hiding this comment.
Thanks for implementing this @spotandjake!
This adds the lsp symbol outline which makes it easier to navigate large files.