Better LSP hover interaction in stack-or-heap #116
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
gated behind [-lsp-compat] flag Signed-off-by: David Vulakh <dvulakh@janestreet.com>
report the entire value binding when not in the lsp-compat regime also move all the lsp-compat tests to a separate file to group them together Signed-off-by: David Vulakh <dvulakh@janestreet.com>
clean up some artifacts of intermediate states to make the total PR diff cleaner Signed-off-by: David Vulakh <dvulakh@janestreet.com>
liam923
approved these changes
Nov 6, 2024
Comment on lines
14
to
17
| let[@tail_mod_cons] rec tails = function | ||
| | hd :: tl -> (hd, tl) :: tails tl | ||
| | [] -> [] | ||
| in |
Contributor
There was a problem hiding this comment.
This function seems a bit more general than necessary - I think it might be a bit clearer if it were something like:
let[@tail_mod_cons] rec with_parents = function
| hd :: next :: tl -> (hd, Some next) :: tails (next :: tl)
| [ hd ] -> [hd, None]
| [] -> []
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds some special cases to
stack-or-heapso that it more consistently reports the same location as atype-enclosingquery at the same position (even if that location less accurately describes the allocating expression). Once the locations coincide in more cases, it will be easier to exposestack-or-heapin a hover-based interface via the LSP.This less precise location reporting is gated behind the
-lsp-compat truecommand-line flag, so editors that query thestack-or-heapRPC directly can still get the maximally descriptive ranges.Here are the two special cases we consider:
stack-or-heapof a (non-polymorphic variant) constructor now reports the location of just the constructor (not the constructed expression) when-lsp-compat trueis passed. This better matches the behavior oftype-enclosing, which e.g. reports'a -> 'a optionfor the type ofSomeinSome 5. Polymorphic variants don't have this behavior intype-enclosing, so we also don't special-case them instack-or-heap.stack-or-heapof an identifier pattern that is the left-hand side of a function binding reports thestack-or-heapof that function. The associated location is the entire binding, unless-lsp-compat trueis passed, in which case it is just the identifier.The new test file
lsp_compat.mldemonstrates these special cases and how the-lps-compatflag affects their treatment.