Repository: tweag/nickel (rust)

Issue:
VSCode Extension should not disregard cursor position and collapse all symbol breadcrumbs to "..." in all occasions
**Describe the bug**
The latest version of `nls` provides proper symbol outlines in the "Outlines" Tab. Normally the same symbol information is also displayed as "breadcrumbs" at the top of the editor view, continuing after the file path and scrolls horizontally if the width exceeds the window (below is an example from a rust file in the nickel codebase, powered by rust-analyzer)

![image](https://github.com/tweag/nickel/assets/812576/058d66d7-f24d-4ed4-9da8-6ee240d0467f)

However with a nickel file, all symbol information are collapsed in "..." after the file path (see below, note the cursor is positioned at `_source2`). While one can always click on "..." and navigate to the symbol, IMHO this behavior greatly diminishes the primary utility of breadcrumb that is providing direct visual guidance to the user without keyboard / mouse interaction.

![image](https://github.com/tweag/nickel/assets/812576/871d4f47-6e3c-40a7-8d2d-c771ea622453)

I searched for vscode setting entries and couldn't find a knob that tweaks this behavior.

**To Reproduce**


**Expected behavior**
Expand the symbol information inline in the breadcrumbs bar.

**Environment**
 - OS name + version: NixOS 23.11
 - Version of the code: `nickel-lang-cli nickel 1.5.0 (rev 13ffc85)`

**Additional context**
Add any other context about the problem here.


Pull request:
Extend the symbol range to include the rhs
Fixes #1886.

The symbols response has two ranges: the "selection range" is just the position of the symbol name, but the "range" is supposed to include the "body" of the symbol. VSCode uses this range for the breadcrumb bar: it shows the symbols in the bar when your cursor is in that symbol's body. Previously, we were just reporting the position of the symbol name for both ranges.

Patch:
diff --git a/lsp/nls/src/requests/symbols.rs b/lsp/nls/src/requests/symbols.rs
index 320e6d56..145912b9 100644
--- a/lsp/nls/src/requests/symbols.rs
+++ b/lsp/nls/src/requests/symbols.rs
@@ -1,5 +1,6 @@
 use lsp_server::{RequestId, Response, ResponseError};
 use lsp_types::{DocumentSymbol, DocumentSymbolParams, SymbolKind};
+use nickel_lang_core::position::RawSpan;
 use nickel_lang_core::term::RichTerm;
 use nickel_lang_core::typ::Type;
 
@@ -36,10 +37,36 @@ fn symbols(
         .flat_map(|rt| {
             rt.fields.into_iter().filter_map(|(id, field)| {
                 let ty = type_lookups.idents.get(&id.into());
-                let (file_id, span) = id.pos.into_opt()?.to_range();
-                let range =
-                    crate::codespan_lsp::byte_span_to_range(world.cache.files(), file_id, span)
-                        .ok()?;
+                let id_pos = id.pos.into_opt()?;
+                let (file_id, id_span) = id_pos.to_range();
+
+                // We need to find the span of the name (that's id_span above), but also the
+                // span of the "whole value," whatever that means. In vscode, there's a little
+                // outline bar at the top that shows you which symbol you're currently in, and it
+                // works by checking whether the cursor is inside the "whole value" range.
+                // We take this range large enough to contain the field value
+                // (if there is one) and any other annotations that we can work
+                // out the positions of.
+                let val_span = field
+                    .metadata
+                    .annotation
+                    .iter()
+                    .filter_map(|ty| ty.typ.pos.into_opt())
+                    .chain(field.value.as_ref().and_then(|val| val.pos.into_opt()))
+                    .fold(id_pos, |a, b| RawSpan::fuse(a, b).unwrap_or(a));
+
+                let selection_range = crate::codespan_lsp::byte_span_to_range(
+                    world.cache.files(),
+                    file_id,
+                    id_span.clone(),
+                )
+                .ok()?;
+                let range = crate::codespan_lsp::byte_span_to_range(
+                    world.cache.files(),
+                    file_id,
+                    val_span.to_range().1,
+                )
+                .ok()?;
 
                 let children = max_depth
                     .checked_sub(1)
@@ -53,7 +80,7 @@ fn symbols(
                     kind: SymbolKind::VARIABLE,
                     tags: None,
                     range,
-                    selection_range: range,
+                    selection_range,
                     children,
                     deprecated: None,
                 })


Test patch:
diff --git a/lsp/nls/tests/inputs/symbols-basic.ncl b/lsp/nls/tests/inputs/symbols-basic.ncl
index fba1a615..98c699aa 100644
--- a/lsp/nls/tests/inputs/symbols-basic.ncl
+++ b/lsp/nls/tests/inputs/symbols-basic.ncl
@@ -9,6 +9,8 @@ let func = fun x => 1 in
   type_checked_block = {
     inner_name = { name = "value" },
   } : _,
+  annotated | String,
+  docced | doc "The symbol range doesn't currently include docs",
 }
 ### [[request]]
 ### type = "Symbols"
diff --git a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__symbols-basic.ncl.snap b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__symbols-basic.ncl.snap
index b9d7dc8f..d69f4ae4 100644
--- a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__symbols-basic.ncl.snap
+++ b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__symbols-basic.ncl.snap
@@ -2,4 +2,4 @@
 source: lsp/nls/tests/main.rs
 expression: output
 ---
-[name (Variable / "String")@3:2-3:6 in 3:2-3:6: [], other_name (Variable / "Dyn")@4:2-4:12 in 4:2-4:12: [inner_name (Variable / "Dyn")@5:4-5:14 in 5:4-5:14: []], type_checked_block (Variable / "Dyn")@7:2-7:20 in 7:2-7:20: [inner_name (Variable / "{ name : String }")@8:4-8:14 in 8:4-8:14: [name (Variable / "String")@8:19-8:23 in 8:19-8:23: []]]]
+[name (Variable / "String")@3:2-3:6 in 3:2-3:16: [], other_name (Variable / "Dyn")@4:2-4:12 in 4:2-6:3: [inner_name (Variable / "Dyn")@5:4-5:14 in 5:4-5:49: []], type_checked_block (Variable / "Dyn")@7:2-7:20 in 7:2-9:7: [inner_name (Variable / "{ name : String }")@8:4-8:14 in 8:4-8:35: [name (Variable / "String")@8:19-8:23 in 8:19-8:33: []]], annotated (Variable / "String")@10:2-10:11 in 10:2-10:20: [], docced (Variable / "Dyn")@11:2-11:8 in 11:2-11:8: []]
diff --git a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__symbols-recursion.ncl.snap b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__symbols-recursion.ncl.snap
index b8263f82..d9ab6630 100644
--- a/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__symbols-recursion.ncl.snap
+++ b/lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__symbols-recursion.ncl.snap
@@ -2,4 +2,4 @@
 source: lsp/nls/tests/main.rs
 expression: output
 ---
-[bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:19: []]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]
+[bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: [bar (Variable / "Dyn")@0:16-0:19 in 0:16-0:25: []]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]