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

Add ability to theme cursor and primary selection #325

Merged
merged 5 commits into from
Jun 23, 2021
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
90 changes: 48 additions & 42 deletions book/src/themes.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,48 +46,54 @@ Possible modifiers:

Possible keys:

| Key | Notes |
| --- | --- |
| `attribute` | |
| `keyword` | |
| `keyword.directive` | Preprocessor directives (\#if in C) |
| `namespace` | |
| `punctuation` | |
| `punctuation.delimiter` | |
| `operator` | |
| `special` | |
| `property` | |
| `variable` | |
| `variable.parameter` | |
| `type` | |
| `type.builtin` | |
| `constructor` | |
| `function` | |
| `function.macro` | |
| `function.builtin` | |
| `comment` | |
| `variable.builtin` | |
| `constant` | |
| `constant.builtin` | |
| `string` | |
| `number` | |
| `escape` | Escaped characters |
| `label` | For lifetimes |
| `module` | |
| `ui.background` | |
| `ui.linenr` | |
| `ui.statusline` | |
| `ui.popup` | |
| `ui.window` | |
| `ui.help` | |
| `ui.text` | |
| `ui.text.focus` | |
| `ui.menu.selected` | |
| `ui.selection` | For selections in the editing area |
| `warning` | LSP warning |
| `error` | LSP error |
| `info` | LSP info |
| `hint` | LSP hint |
| Key | Notes |
| --- | --- |
| `attribute` | |
| `keyword` | |
| `keyword.directive` | Preprocessor directives (\#if in C) |
| `namespace` | |
| `punctuation` | |
| `punctuation.delimiter` | |
| `operator` | |
| `special` | |
| `property` | |
| `variable` | |
| `variable.parameter` | |
| `type` | |
| `type.builtin` | |
| `constructor` | |
| `function` | |
| `function.macro` | |
| `function.builtin` | |
| `comment` | |
| `variable.builtin` | |
| `constant` | |
| `constant.builtin` | |
| `string` | |
| `number` | |
| `escape` | Escaped characters |
| `label` | For lifetimes |
| `module` | |
| `ui.background` | |
| `ui.cursor` | |
| `ui.cursor.insert` | |
| `ui.cursor.select` | |
| `ui.cursor.match` | Matching bracket etc. |
| `ui.linenr` | |
| `ui.statusline` | |
| `ui.statusline.inactive` | |
| `ui.popup` | |
| `ui.window` | |
| `ui.help` | |
| `ui.text` | |
| `ui.text.focus` | |
| `ui.menu.selected` | |
| `ui.selection` | For selections in the editing area |
| `ui.selection.primary` | |
| `warning` | LSP warning |
| `error` | LSP error |
| `info` | LSP info |
| `hint` | LSP hint |

These keys match [tree-sitter scopes](https://tree-sitter.github.io/tree-sitter/syntax-highlighting#theme). We half-follow the common scopes from [macromates language grammars](https://macromates.com/manual/en/language_grammars) with some differences.

Expand Down
48 changes: 35 additions & 13 deletions helix-term/src/ui/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,21 +275,40 @@ impl EditorView {
let end = text.line_to_char(last_line + 1);
Range::new(start, end)
};
let cursor_style = Style::default()
// .bg(Color::Rgb(255, 255, 255))
.add_modifier(Modifier::REVERSED);
let scope = match doc.mode() {
Mode::Insert => "ui.cursor.insert",
Mode::Select => "ui.cursor.select",
Mode::Normal => "ui.cursor",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why shouldn't this be ui.cursor.normal?

Copy link
Member

Choose a reason for hiding this comment

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

Was thinking that too at first. I would actually keep it as ui.cursor, this way the insert and select ones can fall back to it if they're not defined

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was also considering making it ui.cursor.normal but I think as @archseer said it makes more sense as a fallback.

Copy link
Contributor

Choose a reason for hiding this comment

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

But the issue is users would not understand the heuristic if all of us here "think that too at first". Then users have to go through our thought process to know why was the decision made this way. Do you think that is a better tradeoff for visibility vs simplicity?

Copy link
Contributor Author

@vv9k vv9k Jun 21, 2021

Choose a reason for hiding this comment

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

I didn't say I considered it first, I just said I considered it as an option. Actually at first I came up with ui.cursor because that is what for example vim does, you have Cursor and iCursor as highlight groups and not nCursor but perhaps in this case with 3 this is a bit different. I thought vim also had the vCursor but that seems to only be for gui.

Copy link
Member

Choose a reason for hiding this comment

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

I think it makes sense, you're defining a color for ui.cursor but if you'd like to change the style for ui.cursor.insert or ui.cursor.select you can override it. I anticipate most themes should just define a ui.cursor.

};
let cursor_style = theme.try_get(scope).unwrap_or_else(|| {
theme
//if cursor.insert or cursor.select was not present try to default to cursor
.try_get("ui.cursor")
.unwrap_or_else(|| Style::default().add_modifier(Modifier::REVERSED))
});

let selection_style = theme.get("ui.selection");
let primary_style = theme
.try_get("ui.selection.primary")
.unwrap_or(selection_style);
let selection = doc.selection(view.id);
let primary_idx = selection.primary_index();

for selection in doc
.selection(view.id)
for (i, selection) in selection
.iter()
.filter(|range| range.overlaps(&screen))
.enumerate()
.filter(|(_, range)| range.overlaps(&screen))
{
// TODO: render also if only one of the ranges is in viewport
let mut start = view.screen_coords_at_pos(doc, text, selection.anchor);
let mut end = view.screen_coords_at_pos(doc, text, selection.head);

let style = if i != primary_idx {
selection_style
} else {
primary_style
};

let head = end;

if selection.head < selection.anchor {
Expand Down Expand Up @@ -317,7 +336,7 @@ impl EditorView {
),
1,
),
selection_style,
style,
);
} else {
surface.set_style(
Expand All @@ -328,7 +347,7 @@ impl EditorView {
viewport.width.saturating_sub(start.col as u16),
1,
),
selection_style,
style,
);
for i in start.row + 1..end.row {
surface.set_style(
Expand All @@ -339,7 +358,7 @@ impl EditorView {
viewport.width,
1,
),
selection_style,
style,
);
}
surface.set_style(
Expand All @@ -349,7 +368,7 @@ impl EditorView {
(end.col as u16).min(viewport.width),
1,
),
selection_style,
style,
);
}

Expand Down Expand Up @@ -382,9 +401,12 @@ impl EditorView {
if (pos.col as u16) < viewport.width + view.first_col as u16
&& pos.col >= view.first_col
{
let style = Style::default()
.add_modifier(Modifier::REVERSED)
.add_modifier(Modifier::DIM);
let style =
theme.try_get("ui.cursor.match").unwrap_or_else(|| {
Style::default()
.add_modifier(Modifier::REVERSED)
.add_modifier(Modifier::DIM)
});

surface
.get_mut(
Expand Down
3 changes: 2 additions & 1 deletion theme.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@
"ui.text" = { fg = "#a4a0e8" } # lavender
"ui.text.focus" = { fg = "#dbbfef"} # lilac

"ui.selection" = { bg = "#540099" }
"ui.selection" = { bg = "#44258b" }
"ui.selection.primary" = { bg = "#540099" }
"ui.menu.selected" = { fg = "#281733", bg = "#ffffff" } # revolver

"warning" = "#ffcd1c"
Expand Down