Skip to content

Commit

Permalink
fix(exthost): Fix some command / codelens parsing failures (#2993)
Browse files Browse the repository at this point in the history
__Issue:__ Some extensions have codelenses that fail to display in Onivim

__Defect:__ Some extensions will pass a command without an `id` - our parsing logic assumed that an `id` would always be present, and thus, those codelens would fail to decode and not be shown. In addition, some extensions will use labelled icons like `${ellipsis}` in codelens, which we weren't handling.

__Fix:__ 
1) Update our `Command` model to reflect the optional id
2) Use the `Label` renderer for codelens
  • Loading branch information
bryphe authored Jan 15, 2021
1 parent bd1417f commit 7eff3fe
Show file tree
Hide file tree
Showing 9 changed files with 20 additions and 24 deletions.
1 change: 1 addition & 0 deletions CHANGES_CURRENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
- #2978 - Input: Fix `m-` modifier behavior (fixes #2963)
- #2990 - Signature Help: Fix blocking `esc` key press back to normal mode
- #2991 - OSX: Fix shortcut keys double-triggering events
- #2993 - CodeLens: Handle null command id & label icons

### Performance

Expand Down
5 changes: 1 addition & 4 deletions src/Components/Label.re
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@ open Oni_Core;

module Styles = {
open Style;
let text = (~color) => [
textWrap(TextWrapping.NoWrap),
Style.color(color),
];
let text = (~color) => [textWrap(TextWrapping.Wrap), Style.color(color)];
};

let textToElement = (~color, ~font: UiFont.t, ~text) => {
Expand Down
6 changes: 3 additions & 3 deletions src/Exthost/Command.re
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ open Oni_Core;

[@deriving show]
type t = {
id: string,
id: option(string),
label: option(Label.t),
};

Expand All @@ -11,7 +11,7 @@ module Decode = {
Json.Decode.(
obj(({field, _}) =>
{
id: field.required("id", string),
id: field.optional("id", string),
label: field.optional("title", Label.decode),
}
)
Expand All @@ -23,7 +23,7 @@ module Encode = {
open Json.Encode;
let encode = lens =>
obj([
("id", lens.id |> string),
("id", lens.id |> nullable(string)),
("title", lens.label |> nullable(Label.encode)),
]);
};
Expand Down
2 changes: 1 addition & 1 deletion src/Exthost/Exthost.rei
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ module Label: {
module Command: {
[@deriving show]
type t = {
id: string,
id: option(string),
label: option(Label.t),
};

Expand Down
17 changes: 6 additions & 11 deletions src/Feature/LanguageSupport/CodeLens.re
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,11 @@ module View = {
open Revery.UI;
let make = (~leftMargin, ~theme, ~uiFont: Oni_Core.UiFont.t, ~codeLens, ()) => {
let foregroundColor = CodeLensColors.foreground.from(theme);
let text = text(codeLens);
let label =
Exthost.CodeLens.(codeLens.command)
|> OptionEx.flatMap((command: Exthost.Command.t) => command.label)
|> Option.value(~default=Exthost.Label.ofString("(null)"));

<View
style=Style.[
marginTop(4),
Expand All @@ -190,16 +194,7 @@ module View = {
flexGrow(1),
flexShrink(0),
]>
<Text
text
fontFamily={uiFont.family}
fontSize={uiFont.size}
style=Style.[
color(foregroundColor),
flexGrow(1),
textWrap(Revery.TextWrapping.Wrap),
]
/>
<Oni_Components.Label font=uiFont color=foregroundColor label />
</View>;
};
};
2 changes: 1 addition & 1 deletion src/Service/Exthost/Service_Exthost.re
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ module Internal = {
~versionId=version,
~lines,
~modeId,
~isDirty=true,
~isDirty=false,
Uri.fromPath(filePath),
);
});
Expand Down
2 changes: 1 addition & 1 deletion src/Store/ExtensionClient.re
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ let create =
}),
) =>
let command =
command |> Option.map(({id, _}: Exthost.Command.t) => id);
command |> OptionEx.flatMap(({id, _}: Exthost.Command.t) => id);
dispatch(
Actions.StatusBar(
Feature_StatusBar.ItemAdded(
Expand Down
4 changes: 2 additions & 2 deletions test/Exthost/LanguageFeaturesTest.re
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe("LanguageFeaturesTest", ({describe, _}) => {
Some(
Exthost.Label.ofString("codelens: command1"),
),
id: "codelens.command1",
id: Some("codelens.command1"),
},
),
},
Expand All @@ -110,7 +110,7 @@ describe("LanguageFeaturesTest", ({describe, _}) => {
Some(
Exthost.Label.ofString("codelens: command2"),
),
id: "codelens.command2",
id: Some("codelens.command2"),
},
),
},
Expand Down
5 changes: 4 additions & 1 deletion test/Feature/Editor/EditorTests.re
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ describe("Editor", ({describe, _}) => {
},
command:
Some(
Command.{id: uniqueId, label: Some(Label.ofString(uniqueId))},
Command.{
id: Some(uniqueId),
label: Some(Label.ofString(uniqueId)),
},
),
}
);
Expand Down

0 comments on commit 7eff3fe

Please sign in to comment.