Skip to content

Commit

Permalink
fix(hover/#3231): Part 1 - Hover not showing up in some cases (#3390)
Browse files Browse the repository at this point in the history
__Issue:__ For some extensions, diagnostics would show the squiggly underline the editor, but wouldn't show any details in the hover UI (like in #3231 )

__Defect:__ Some extensions don't send a diagnostic range - they send a position, and the editor is supposed to treat the diagnostic as spanning the _token_ at the range. We were handling that correctly for rendering, but for the hover, Onivim wasn't detecting positions as being in that implicit range.

__Fix:__ Handle hover in this case - find all diagnostics in the token range, and show in the hover ui.
  • Loading branch information
bryphe authored Apr 11, 2021
1 parent 65c7ab2 commit 41c412d
Show file tree
Hide file tree
Showing 15 changed files with 248 additions and 187 deletions.
1 change: 1 addition & 0 deletions CHANGES_CURRENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
- #3382 - Input: Handle `<capslock>` key
- #3389 - Editor: Render diagnostics with squiggly lines (fixes #2827)
- #3394 - Extensions: Fix error parsing extension manifest with boolean when express (related #3388)
- #3390 - Diagnostics: Some diagnostics wouldn't show in hover UI (related #3231)

### Performance

Expand Down
41 changes: 39 additions & 2 deletions src/Core/Buffer.re
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,45 @@ let characterRangeAt = (line, buffer) => {
});
};

let getNumberOfLines = (buffer: t) => Array.length(buffer.lines);

let tokenAt = (~languageConfiguration, position: CharacterPosition.t, buffer) => {
let line = position.line;
let character = position.character;
let lineNumber = line |> EditorCoreTypes.LineNumber.toZeroBased;
let numberOfLines = getNumberOfLines(buffer);

if (lineNumber < 0 || lineNumber >= numberOfLines) {
None;
} else {
let bufferLine = getLine(lineNumber, buffer);
let f = uchar =>
LanguageConfiguration.isWordCharacter(uchar, languageConfiguration);
let startIndex =
BufferLine.traverse(
~f,
~direction=`Backwards,
~index=character,
bufferLine,
)
|> Option.value(~default=character);
let stopIndex =
BufferLine.traverse(
~f,
~direction=`Forwards,
~index=character,
bufferLine,
)
|> Option.value(~default=character);
Some(
CharacterRange.{
start: CharacterPosition.{line, character: startIndex},
stop: CharacterPosition.{line, character: stopIndex},
},
);
};
};

let lastLine = buffer => {
buffer.lines |> Array.length |> max(1) |> LineNumber.ofOneBased;
};
Expand Down Expand Up @@ -245,8 +284,6 @@ let getUri = (buffer: t) => {
};
};

let getNumberOfLines = (buffer: t) => Array.length(buffer.lines);

let characterRange = (buffer: t) => {
let start =
CharacterPosition.{line: LineNumber.zero, character: CharacterIndex.zero};
Expand Down
4 changes: 4 additions & 0 deletions src/Core/Buffer.rei
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ let hasTrailingNewLine: t => bool;
let rawLine: (LineNumber.t, t) => option(string);
let characterRangeAt: (LineNumber.t, t) => option(CharacterRange.t);

let tokenAt:
(~languageConfiguration: LanguageConfiguration.t, CharacterPosition.t, t) =>
option(CharacterRange.t);

let lastLine: t => LineNumber.t;

let getVersion: t => int;
Expand Down
6 changes: 6 additions & 0 deletions src/Feature/Diagnostics/Feature_Diagnostics.re
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,12 @@ let getDiagnosticsAtPosition = (instance, buffer, position) => {
CharacterRange.contains(position, range)
);
};
let getDiagnosticsInRange = (model, buffer, query) => {
getDiagnostics(model, buffer)
|> List.filter((Diagnostic.{range, _}) =>
CharacterRange.intersects(query, range)
);
};

let getDiagnosticsMap = (instance, buffer) => {
getDiagnostics(instance, buffer) |> Internal.explodeDiagnostics(buffer);
Expand Down
2 changes: 2 additions & 0 deletions src/Feature/Diagnostics/Feature_Diagnostics.rei
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ let moveMarkers:
let getDiagnostics: (model, Buffer.t) => list(Diagnostic.t);
let getDiagnosticsAtPosition:
(model, Buffer.t, CharacterPosition.t) => list(Diagnostic.t);
let getDiagnosticsInRange:
(model, Buffer.t, CharacterRange.t) => list(Diagnostic.t);
let getDiagnosticsMap: (model, Buffer.t) => IntMap.t(list(Diagnostic.t));

let getAllDiagnostics: model => list((Uri.t, Diagnostic.t));
35 changes: 2 additions & 33 deletions src/Feature/Editor/Editor.re
Original file line number Diff line number Diff line change
Expand Up @@ -1338,39 +1338,8 @@ let getLeftVisibleColumn = view => {
};

let getTokenAt =
(~languageConfiguration, {line, character}: CharacterPosition.t, editor) => {
let lineNumber = line |> EditorCoreTypes.LineNumber.toZeroBased;

if (lineNumber < 0
|| lineNumber >= EditorBuffer.numberOfLines(editor.buffer)) {
None;
} else {
let bufferLine = EditorBuffer.line(lineNumber, editor.buffer);
let f = uchar =>
LanguageConfiguration.isWordCharacter(uchar, languageConfiguration);
let startIndex =
BufferLine.traverse(
~f,
~direction=`Backwards,
~index=character,
bufferLine,
)
|> Option.value(~default=character);
let stopIndex =
BufferLine.traverse(
~f,
~direction=`Forwards,
~index=character,
bufferLine,
)
|> Option.value(~default=character);
Some(
CharacterRange.{
start: CharacterPosition.{line, character: startIndex},
stop: CharacterPosition.{line, character: stopIndex},
},
);
};
(~languageConfiguration, position: CharacterPosition.t, editor) => {
EditorBuffer.tokenAt(~languageConfiguration, position, editor.buffer);
};

let getContentPixelWidth = editor => {
Expand Down
2 changes: 2 additions & 0 deletions src/Feature/Editor/EditorBuffer.re
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,5 @@ let hasLine = (lineNumber, buffer) => {
let lineCount = numberOfLines(buffer);
lineIdx >= 0 && lineIdx < lineCount;
};

let tokenAt = Oni_Core.Buffer.tokenAt;
14 changes: 10 additions & 4 deletions src/Feature/Editor/EditorBuffer.rei
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
open EditorCoreTypes;
open Oni_Core;
// EditorBuffer is a subset of the buffer model,
// specifically containing the functions and data
// needed by the editor surface.

type t;

let ofBuffer: Oni_Core.Buffer.t => t;
let ofBuffer: Buffer.t => t;
let id: t => int;
let getEstimatedMaxLineLength: t => int;
let numberOfLines: t => int;
let line: (int, t) => Oni_Core.BufferLine.t;
let line: (int, t) => BufferLine.t;
let hasLine: (EditorCoreTypes.LineNumber.t, t) => bool;
let font: t => Oni_Core.Font.t;
let fileType: t => Oni_Core.Buffer.FileType.t;
let font: t => Font.t;
let fileType: t => Buffer.FileType.t;
let measure: (Uchar.t, t) => float;

let tokenAt:
(~languageConfiguration: LanguageConfiguration.t, CharacterPosition.t, t) =>
option(CharacterRange.t);
3 changes: 1 addition & 2 deletions src/Feature/Editor/EditorSurface.re
Original file line number Diff line number Diff line change
Expand Up @@ -278,11 +278,10 @@ let%component make =
~uiFont,
~editorFont,
~model=languageSupport,
~diagnostics,
~tokenTheme,
~grammars=grammarRepository,
~buffer,
~editorId=Some(editorId),
~editorId,
~languageInfo,
);

Expand Down
5 changes: 3 additions & 2 deletions src/Feature/LanguageSupport/Feature_LanguageSupport.re
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ module Msg = {
let update =
(
~config,
~diagnostics,
~extensions,
~languageConfiguration,
~maybeSelection,
Expand Down Expand Up @@ -488,7 +489,9 @@ let update =
| Hover(hoverMsg) =>
let (hover', outMsg) =
Hover.update(
~languageConfiguration,
~cursorLocation,
~diagnostics,
~maybeBuffer,
~editorId,
~extHostClient=client,
Expand Down Expand Up @@ -897,7 +900,6 @@ module Hover = {
module Popup = {
let make =
(
~diagnostics,
~theme,
~tokenTheme,
~languageInfo,
Expand All @@ -910,7 +912,6 @@ module Hover = {
) => {
let {hover, _} = model;
OldHover.Popup.make(
~diagnostics,
~theme,
~tokenTheme,
~languageInfo,
Expand Down
4 changes: 2 additions & 2 deletions src/Feature/LanguageSupport/Feature_LanguageSupport.rei
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ type outmsg =
let update:
(
~config: Oni_Core.Config.resolver,
~diagnostics: Feature_Diagnostics.model,
~extensions: Feature_Extensions.model,
~languageConfiguration: Oni_Core.LanguageConfiguration.t,
~maybeSelection: option(CharacterRange.t),
Expand Down Expand Up @@ -283,7 +284,6 @@ module Hover: {
module Popup: {
let make:
(
~diagnostics: Feature_Diagnostics.model,
~theme: Oni_Core.ColorTheme.Colors.t,
~tokenTheme: Oni_Syntax.TokenTheme.t,
~languageInfo: Exthost.LanguageInfo.t,
Expand All @@ -292,7 +292,7 @@ module Hover: {
~grammars: Oni_Syntax.GrammarRepository.t,
~model: model,
~buffer: Oni_Core.Buffer.t,
~editorId: option(int)
~editorId: int
) =>
option((CharacterPosition.t, list(Oni_Components.Popup.Section.t)));
};
Expand Down
Loading

0 comments on commit 41c412d

Please sign in to comment.