Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editor: Themable cursor #1373

Merged
merged 8 commits into from
Feb 27, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/Core/Theme.re
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ type t = {
activityBarForeground: Color.t,
editorBackground: Color.t,
editorForeground: Color.t,
editorCursorBackground: Color.t,
editorCursorForeground: Color.t,
editorHoverWidgetBackground: Color.t,
editorHoverWidgetBorder: Color.t,
editorLineHighlightBackground: Color.t,
Expand Down Expand Up @@ -135,6 +137,8 @@ let default: t = {
activityBarForeground: Color.hex("#DCDCDC"),
editorBackground: Color.hex("#2F3440"),
editorForeground: Color.hex("#DCDCDC"),
editorCursorBackground: Color.hex("#2F3440"),
editorCursorForeground: Color.hex("#DCDCDC"),
editorFindMatchBackground: Color.hex("#42557b"),
editorFindMatchBorder: Color.hex("#457dff"),
editorFindMatchHighlightBackground: Color.hex("#314365"),
Expand Down Expand Up @@ -223,6 +227,17 @@ let ofColorTheme = (uiTheme, ct: Textmate.ColorTheme.t) => {
let editorForeground =
getColor(defaultForeground, ["editor.foreground", "foreground"]);

let editorCursorBackground =
getColor(
defaultBackground,
["editorCursor.background", "editor.background", "background"],
);
let editorCursorForeground =
getColor(
defaultForeground,
["editorCursor.foreground", "editor.foreground", "foreground"],
);

let editorHoverWidgetBackground =
getColor(
defaultBackground,
Expand Down Expand Up @@ -439,6 +454,8 @@ let ofColorTheme = (uiTheme, ct: Textmate.ColorTheme.t) => {
activityBarForeground,
editorBackground,
editorForeground,
editorCursorBackground,
editorCursorForeground,
editorHoverWidgetBackground,
editorHoverWidgetBorder,
editorIndentGuideBackground,
Expand Down
85 changes: 35 additions & 50 deletions src/Feature/Editor/CursorView.re
Original file line number Diff line number Diff line change
@@ -1,68 +1,53 @@
open EditorCoreTypes;
open Revery;
open Revery.UI;

open Oni_Core;

let make =
let render =
(
~context: Draw.context,
~buffer,
~mode: Vim.Mode.t,
~isActiveSplit,
~editorFont: EditorFont.t,
~cursorPosition: Location.t,
~editor: Editor.t,
(),
~theme: Theme.t,
) => {
let cursorLine = Index.toZeroBased(cursorPosition.line);
let line = Index.toZeroBased(cursorPosition.line);
let column = Index.toZeroBased(cursorPosition.column);
let bufferLine = Buffer.getLine(line, buffer);
let lineCount = Buffer.getNumberOfLines(buffer);

let (cursorOffset, cursorCharacterWidth) =
if (lineCount > 0 && cursorLine < lineCount) {
let cursorLine = Buffer.getLine(cursorLine, buffer);

let (cursorOffset, width) =
BufferViewTokenizer.getCharacterPositionAndWidth(
cursorLine,
Index.toZeroBased(cursorPosition.column),
);
(cursorOffset, width);
let (offset, characterWidth) =
if (lineCount > 0 && line < lineCount) {
BufferViewTokenizer.getCharacterPositionAndWidth(bufferLine, column);
} else {
(0, 1);
};

let fullCursorWidth =
cursorCharacterWidth * int_of_float(editorFont.measuredWidth);

let cursorWidth =
switch (mode, isActiveSplit) {
| (Insert, true) => 2
| _ => fullCursorWidth
let x = float(offset) *. context.charWidth;
let y = float(line) *. context.lineHeight +. 0.5;
let height = context.lineHeight;
let background = theme.editorCursorBackground;
let foreground = theme.editorCursorForeground;

switch (mode, isActiveSplit) {
| (Insert, true) =>
let width = 2.;
Draw.rect(~context, ~x, ~y, ~width, ~height, ~color=foreground);

| _ =>
let width = float(characterWidth) *. context.charWidth;
Draw.rect(~context, ~x, ~y, ~width, ~height, ~color=foreground);

switch (BufferLine.subExn(~index=offset, ~length=1, bufferLine)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For grabbing the character - we actually want ~index=cursorPosition.column instead of offset.

There's actually 3 'metrics' for character position:

  • The byte position of a character. This is the actual byte position in the string.
  • The UTF8-index of a character. UTF-8 characters have a variable length - from 1-4 bytes. So if you have two 4-byte characters - the first character would be at byte index 0, character index 0, and the second would be at byte index 4, character index 1. Most of the BufferLine APIs use the UTF8-index of the character.
  • The width (and offset) of a character. These are like the UTF-8 index, except they account for the visual width and visual offset of a character. For example, a tab character is only one byte, but depending on the settings, it could take up a variable amount of visual space. The width is the width of the individual character, and the offset is the x-position of the character, taking into account all the previous character's widths before it.

If we pass in the offset to subExn - we get this result with a tab character:
bug-cursor-overlay

Note how when the cursor is over the c, it's actually showing s - that's because the offset is 4 (due to the visual space of the tab), but we're asking for the character index at position 4 - which would be the s: |\t|c|o|n|s|.

An extra point of confusion is that the cursorPosition.column is a byte position coming from Vim (`github.com/onivim/reason-libvim/pull/90)....

Lot of opportunity for clean up here... It'd be nice to have something in place like we have for the zero-based index vs the one-based index - a way to leverage the type system to help us manage these different 'views' into the UTF-8 string, and not getting any 'parity mismatches' where we pass something like a byte offset into an API that expects a UTF-8 character index.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, and thanks for the explanation! I find it extra confusing that subExn then takes the position in bytes, but the length is in characters. So I have to mix and match even for a single function call.

Anyway, fixed in 5adf1e8

| text =>
Draw.shapedText(
~context,
~x=x -. 0.5,
~y=y -. context.fontMetrics.ascent -. 0.5,
~color=background,
text,
)
| exception _ => ()
};

let cursorPixelY =
int_of_float(
editorFont.measuredHeight
*. float(Index.toZeroBased(cursorPosition.line))
-. editor.scrollY
+. 0.5,
);

let cursorPixelX =
int_of_float(
editorFont.measuredWidth *. float(cursorOffset) -. editor.scrollX +. 0.5,
);

let style =
Style.[
position(`Absolute),
top(cursorPixelY),
left(cursorPixelX),
height(int_of_float(editorFont.measuredHeight)),
width(cursorWidth),
backgroundColor(Colors.white),
];
let cursorOpacity = isActiveSplit ? 0.5 : 0.25;

<Opacity opacity=cursorOpacity> <View style /> </Opacity>;
};
};
10 changes: 9 additions & 1 deletion src/Feature/Editor/SurfaceView.re
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,17 @@ let%component make =
indentation,
);
};

CursorView.render(
~context,
~buffer,
~mode,
~isActiveSplit,
~cursorPosition,
~theme,
);
}}
/>
<CursorView buffer mode isActiveSplit cursorPosition editor editorFont />
<View style=Styles.horizontalScrollBar>
<EditorHorizontalScrollbar
editor
Expand Down