fix: off-by-one hover range result

Fix sourcegraph/sourcegraph#1417. The hover result was not being properly translated from 0-indexed to 1-indexed.
sourcegraph · Dec 14, 2018 · a327422 · a327422
1 parent 7ed22c7
commit a327422
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 3 deletions.
diff --git a/src/hoverifier.test.ts b/src/hoverifier.test.ts
@@ -36,6 +36,7 @@ describe('Hoverifier', () => {
 
             const delayTime = 100
             const hoverRange = { start: { line: 1, character: 2 }, end: { line: 3, character: 4 } }
+            const hoverRange1Indexed = { start: { line: 2, character: 3 }, end: { line: 4, character: 5 } }
 
             scheduler.run(({ cold, expectObservable }) => {
                 const hoverifier = createHoverifier({
@@ -82,7 +83,7 @@ describe('Hoverifier', () => {
                     [key: string]: Range | undefined
                 } = {
                     a: undefined, // highlightedRange is undefined when the hover is loading
-                    b: hoverRange,
+                    b: hoverRange1Indexed,
                 }
 
                 // Hover over https://sourcegraph.sgdev.org/github.com/gorilla/mux@cb4698366aa625048f3b815af6a0dea8aef9280a/-/blob/mux.go#L24:6

diff --git a/src/hoverifier.ts b/src/hoverifier.ts
@@ -615,7 +615,7 @@ export function createHoverifier<C extends object>({
                         return of({ hoverOrError, position: undefined as Position | undefined, ...rest })
                     }
 
-                    // LSP is 0-indexed, the code here is currently 1-indexed
+                    // The requested position is is 0-indexed; the code here is currently 1-indexed
                     const { line, character } = pos
                     pos = { line: line + 1, character: character + 1, ...pos }
 
@@ -642,7 +642,17 @@ export function createHoverifier<C extends object>({
                 let highlightedRange: Range | undefined
                 if (hoverOrError && !isErrorLike(hoverOrError) && hoverOrError !== LOADING) {
                     if (hoverOrError.range) {
-                        highlightedRange = hoverOrError.range
+                        // The result is 0-indexed; the code view is treated as 1-indexed.
+                        highlightedRange = {
+                            start: {
+                                line: hoverOrError.range.start.line + 1,
+                                character: hoverOrError.range.start.character + 1,
+                            },
+                            end: {
+                                line: hoverOrError.range.end.line + 1,
+                                character: hoverOrError.range.end.character + 1,
+                            },
+                        }
                     } else if (position) {
                         highlightedRange = { start: position, end: position }
                     }