feat: add optional position adjuster to HoverifierOptions

src/hoverifier.test.ts

@@ -6,7 +6,13 @@ import { Position } from 'vscode-languageserver-types'
 
 import { noop } from 'lodash'
 import { propertyIsDefined } from './helpers'
-import { createHoverifier, LOADER_DELAY, TOOLTIP_DISPLAY_DELAY } from './hoverifier'
+import {
+    AdjustmentDirection,
+    createHoverifier,
+    LOADER_DELAY,
+    PositionAdjuster,
+    TOOLTIP_DISPLAY_DELAY,
+} from './hoverifier'
 import { HoverOverlayProps } from './HoverOverlay'
 import { findPositionsFromEvents } from './positions'
 import { CodeViewProps, DOM } from './testutils/dom'
@@ -108,4 +114,83 @@ describe('Hoverifier', () => {
             })
         }
     })
+
+    /**
+     * This test ensures that the adjustPosition options is being called in the ways we expect. This test is actually not the best way to ensure the feature
+     * works as expected. This is a good example of a bad side effect of how the main `hoverifier.ts` file is too tightly integrated with itself. Ideally, I'd be able to assert
+     * that the effected positions have actually been adjusted as intended but this is impossible with the current implementation. We can assert that the `HoverFetcher` and `JumpURLFetcher`s
+     * have the adjusted positions (AdjustmentDirection.CodeViewToActual). However, we cannot reliably assert that the code "highlighting" the token has the position adjusted (AdjustmentDirection.ActualToCodeView).
+     */
+    it('PositionAdjuster gets called when expected', () => {
+        for (const codeView of testcases) {
+            const scheduler = new TestScheduler((a, b) => chai.assert.deepEqual(a, b))
+
+            scheduler.run(({ cold, expectObservable }) => {
+                const adjustmentDirections = new Subject<AdjustmentDirection>()
+
+                const fetchHover = createStubHoverFetcher({})
+                const fetchJumpURL = createStubJumpURLFetcher('def')
+
+                const adjustPosition: PositionAdjuster = ({ direction, position }) => {
+                    adjustmentDirections.next(direction)
+
+                    return of(position)
+                }
+
+                const hoverifier = createHoverifier({
+                    closeButtonClicks: new Observable<MouseEvent>(),
+                    goToDefinitionClicks: new Observable<MouseEvent>(),
+                    hoverOverlayElements: of(null),
+                    hoverOverlayRerenders: EMPTY,
+                    fetchHover,
+                    fetchJumpURL,
+                    pushHistory: noop,
+                    logTelemetryEvent: noop,
+                })
+
+                const positionJumps = new Subject<{
+                    position: Position
+                    codeView: HTMLElement
+                    scrollElement: HTMLElement
+                }>()
+
+                const positionEvents = of(codeView.codeView).pipe(findPositionsFromEvents(codeView))
+
+                const subscriptions = new Subscription()
+
+                subscriptions.add(hoverifier)
+                subscriptions.add(
+                    hoverifier.hoverify({
+                        dom: codeView,
+                        positionEvents,
+                        positionJumps,
+                        adjustPosition,
+                        resolveContext: () => codeView.revSpec,
+                    })
+                )
+
+                const inputDiagram = 'ab'
+                // There is probably a bug in code that is unrelated to this feature that is causing the PositionAdjuster to be called an extra time.
+                // It should look like '-(ba)'. That is, we adjust the position from CodeViewToActual for the LSP fetches and then back from CodeViewToActual
+                // for highlighting the token in the DOM.
+                const outputDiagram = 'a(ba)'
+
+                const outputValues: {
+                    [key: string]: AdjustmentDirection
+                } = {
+                    a: AdjustmentDirection.ActualToCodeView,
+                    b: AdjustmentDirection.CodeViewToActual,
+                }
+
+                cold(inputDiagram).subscribe(() =>
+                    clickPositionImpure(codeView, {
+                        line: 1,
+                        character: 1,
+                    })
+                )
+
+                expectObservable(adjustmentDirections).toBe(outputDiagram, outputValues)
+            })
+        }
+    })
 })

src/hoverifier.ts

@@ -115,11 +115,37 @@ export interface PositionJump {
     scrollElement: HTMLElement
 }
 
+/**
+ * The possible directions to adjust a position in.
+ */
+export enum AdjustmentDirection {
+    /** Adjusting the position from what is found on the page to what it would be in the actual file. */
+    CodeViewToActual,
+    /** Adjusting the position from what is in the actual file to what would be found on the page. */
+    ActualToCodeView,
+}
+
+export interface AdjustPositionProps {
+    /** The code view the token is in. */
+    codeView: HTMLElement
+    /** The position the token is at. */
+    position: HoveredToken & HoveredTokenContext
+    /** The direction the adjustment should go. */
+    direction: AdjustmentDirection
+}
+
+/**
+ * Function to adjust positions coming into and out of hoverifier. It can be used to correct the position used in HoverFetcher and
+ * JumpURLFetcher requests and the position of th etoken to highlight in the code view. This is useful for code hosts that convert whitespace.
+ */
+export type PositionAdjuster = (props: AdjustPositionProps) => Observable<Position>
+
 /**
  * HoverifyOptions that need to be included internally with every event
  */
 export interface EventOptions {
     resolveContext: ContextResolver
+    adjustPosition?: PositionAdjuster
     dom: DOMFunctions
 }
 
@@ -296,6 +322,24 @@ export const createHoverifier = ({
         debounceTime(50),
         // Do not consider mouseovers while overlay is pinned
         filter(() => !container.values.hoverOverlayIsFixed),
+        switchMap(
+            ({ adjustPosition, codeView, resolveContext, position, ...rest }) =>
+                adjustPosition && position
+                    ? adjustPosition({
+                          codeView,
+                          position: { ...position, ...resolveContext(position) },
+                          direction: AdjustmentDirection.CodeViewToActual,
+                      }).pipe(
+                          map(({ line, character }) => ({
+                              codeView,
+                              resolveContext,
+                              position: { ...position, line, character },
+                              adjustPosition,
+                              ...rest,
+                          }))
+                      )
+                    : of({ adjustPosition, codeView, resolveContext, position, ...rest })
+        ),
         share()
     )
 
@@ -305,6 +349,24 @@ export const createHoverifier = ({
             target: event.target as HTMLElement,
             ...rest,
         })),
+        switchMap(
+            ({ adjustPosition, codeView, resolveContext, position, ...rest }) =>
+                adjustPosition && position
+                    ? adjustPosition({
+                          codeView,
+                          position: { ...position, ...resolveContext(position) },
+                          direction: AdjustmentDirection.CodeViewToActual,
+                      }).pipe(
+                          map(({ line, character }) => ({
+                              codeView,
+                              resolveContext,
+                              position: { ...position, line, character },
+                              adjustPosition,
+                              ...rest,
+                          }))
+                      )
+                    : of({ adjustPosition, codeView, resolveContext, position, ...rest })
+        ),
         share()
     )
 
@@ -370,7 +432,13 @@ export const createHoverifier = ({
     const hoverObservables = resolvedPositions.pipe(
         map(({ position, ...rest }) => {
             if (!position) {
-                return of({ ...rest, hoverOrError: null, part: undefined })
+                return of({
+                    // Typescript seems to give up on type inference if we don't explicitely declare the types here.
+                    hoverOrError: null as 'loading' | HoverMerged | Error | null | undefined,
+                    position: undefined as (HoveredToken & HoveredTokenContext) | undefined,
+                    part: undefined,
+                    ...rest,
+                })
             }
             // Fetch the hover for that position
             const hoverFetch = fetchHover(position).pipe(
@@ -392,15 +460,46 @@ export const createHoverifier = ({
                     takeUntil(hoverFetch)
                 ),
                 hoverFetch
-            ).pipe(map(hoverOrError => ({ ...rest, hoverOrError, part: position.part })))
+            ).pipe(
+                map(hoverOrError => ({
+                    ...rest,
+                    position,
+                    hoverOrError,
+                    part: position.part,
+                }))
+            )
         }),
         share()
     )
     // Highlight the hover range returned by the language server
     subscription.add(
         hoverObservables
-            .pipe(switchMap(hoverObservable => hoverObservable))
-            .subscribe(({ hoverOrError, codeView, dom, part }) => {
+            .pipe(
+                switchMap(hoverObservable => hoverObservable),
+                switchMap(({ hoverOrError, position, adjustPosition, ...rest }) => {
+                    if (!HoverMerged.is(hoverOrError) || !hoverOrError.range || !position) {
+                        return of({ hoverOrError, position: undefined as Position | undefined, ...rest })
+                    }
+
+                    // LSP is 0-indexed, the code here is currently 1-indexed
+                    const { line, character } = hoverOrError.range.start
+                    const pos = { line: line + 1, character: character + 1, ...position }
+
+                    if (!adjustPosition) {
+                        return of({ hoverOrError, position: pos, ...rest })
+                    }
+
+                    return adjustPosition({
+                        codeView: rest.codeView,
+                        direction: AdjustmentDirection.ActualToCodeView,
+                        position: {
+                            ...position,
+                            part: rest.part,
+                        },
+                    }).pipe(map(position => ({ position, hoverOrError, ...rest })))
+                })
+            )
+            .subscribe(({ hoverOrError, position, codeView, dom, part }) => {
                 container.update({
                     hoverOrError,
                     // Reset the hover position, it's gonna be repositioned after the hover was rendered
@@ -410,12 +509,11 @@ export const createHoverifier = ({
                 if (currentHighlighted) {
                     currentHighlighted.classList.remove('selection-highlight')
                 }
-                if (!HoverMerged.is(hoverOrError) || !hoverOrError.range) {
+                if (!position) {
                     return
                 }
-                // LSP is 0-indexed, the code here is currently 1-indexed
-                const { line, character } = hoverOrError.range.start
-                const token = getTokenAtPosition(codeView, { line: line + 1, character: character + 1 }, dom, part)
+
+                const token = getTokenAtPosition(codeView, position, dom, part)
                 if (!token) {
                     return
                 }