Skip to content

codemirror file view: Add "click to go to definition" and other fixes

Administrator requested to merge fkling/cm-file-hovercard-improvements into main

Created by: fkling

This commit

  • adds support for "click to go to definition"
  • hides the caret
  • fixes hover token/active range highlighting
  • hide close button for non-pinned hovercards

Click to go to definition

Normally the WebHoverOverlay expects to be passed an element to which it can attach a click event handler to provide this functionality. With CodeMirror we normally to not operate on the DOM level. So I extended WebHoverOverlay to accept an alternative source for click events. When the CodeMirror extenion creates a hovercard, it also registers a callback that should be called if a click happens within the active range (see also the third point below).

Hide caret

The behavior of "click to go to definition" seems to be at odds with having a real caret. Having a crate is actually a side effect of making the blob view focusable and thus being able to trigger CodeMirror's search functionality. While a caret might be useful if/when we make the blob view more "IDE like", for the time being we don't have an actual use for it. This commit hides the caret (and disables some basic cursor movement keys) to achieve parity with the current blob view.

I considered using CodeMirror's drawSelection extension which lets CodeMirror take control over caret and selection rendering, but this selection rendering interfered with rendering selected lines (text selection was not visible in selected lines).

Fix highlighting for active ranges

Until now only the word underneath the cursor was highlighted, as determined by CodeMirror. However, the requested hover information also includes a range to which this information applies. This range should be used for highlighting and for click to jump to definition behavior. A couple of places that work with the hover ranges needed to be updated to account for that.

Hide close button for non-pinned hovercards

The close button wasn't functional for non-pinned hovercards and it's arguable whether it should exist at all. This commit only shows the close button when the hovercard is pinned.

Befor/after demo:

https://user-images.githubusercontent.com/179026/184025905-ab9beac6-3f31-4575-aad4-dfee003a2805.mp4

Test plan

  • Disable CodeMirror file view (if enabled). Go to a file, click to go to definition should work as before.
  • Enable CodeMirror file view. Open a Go file and hover over a package import:
    • Close button should not be visible
    • The full import path should be highlighted, not just the word underneath the cursor
    • Moving the mouse over the highlighted path should not let the hovercard flicker
    • Clicking on the path should go to its implementation
    • Clicking on another location in a file (that doesn't trigger go to definition) should not show a caret.
    • Pressing up,down,left or right arrow should not cause the document to scroll (due to moving the invisible caret)
    • Selecting text does not trigger "click to go to definition"

App preview:

Check out the client app preview documentation to learn more.

Merge request reports