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.

Sign up for GitHub

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

Jump to bottom

completions: Add colors based on the LSP item kind #1905

Closed
wants to merge 1 commit into from
Closed

completions: Add colors based on the LSP item kind #1905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 5 additions & 5 deletions helix-term/src/commands.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use helix_view::{
keyboard::KeyCode,
tree,
view::View,
Document, DocumentId, Editor, ViewId,
Document, DocumentId, Editor, Theme, ViewId,
};

use anyhow::{anyhow, bail, ensure, Context as _};
Expand Down Expand Up @@ -1989,7 +1989,7 @@ fn global_search(cx: &mut Context) {
impl ui::menu::Item for FileResult {
type Data = Option<PathBuf>;

fn format(&self, current_path: &Self::Data) -> Row {
fn format(&self, current_path: &Self::Data, _theme: Option<&Theme>) -> Row {
let relative_path = helix_core::path::get_relative_path(&self.path)
.to_string_lossy()
.into_owned();
Expand Down Expand Up @@ -2459,7 +2459,7 @@ fn buffer_picker(cx: &mut Context) {
impl ui::menu::Item for BufferMeta {
type Data = ();

fn format(&self, _data: &Self::Data) -> Row {
fn format(&self, _data: &Self::Data, _theme: Option<&Theme>) -> Row {
let path = self
.path
.as_deref()
Expand Down Expand Up @@ -2523,7 +2523,7 @@ fn jumplist_picker(cx: &mut Context) {
impl ui::menu::Item for JumpMeta {
type Data = ();

fn format(&self, _data: &Self::Data) -> Row {
fn format(&self, _data: &Self::Data, _theme: Option<&Theme>) -> Row {
let path = self
.path
.as_deref()
Expand Down Expand Up @@ -2603,7 +2603,7 @@ fn jumplist_picker(cx: &mut Context) {
impl ui::menu::Item for MappableCommand {
type Data = ReverseKeymap;

fn format(&self, keymap: &Self::Data) -> Row {
fn format(&self, keymap: &Self::Data, _theme: Option<&Theme>) -> Row {
let fmt_binding = |bindings: &Vec<Vec<KeyEvent>>| -> String {
bindings.iter().fold(String::new(), |mut acc, bind| {
if !acc.is_empty() {
Expand Down
8 changes: 4 additions & 4 deletions helix-term/src/commands/dap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use dap::{StackFrame, Thread, ThreadStates};
use helix_core::syntax::{DebugArgumentValue, DebugConfigCompletion, DebugTemplate};
use helix_dap::{self as dap, Client};
use helix_lsp::block_on;
use helix_view::editor::Breakpoint;
use helix_view::{editor::Breakpoint, Theme};

use serde_json::{to_value, Value};
use tokio_stream::wrappers::UnboundedReceiverStream;
Expand All @@ -25,23 +25,23 @@ use helix_view::handlers::dap::{breakpoints_changed, jump_to_stack_frame, select
impl ui::menu::Item for StackFrame {
type Data = ();

fn format(&self, _data: &Self::Data) -> Row {
fn format(&self, _data: &Self::Data, _theme: Option<&Theme>) -> Row {
self.name.as_str().into() // TODO: include thread_states in the label
}
}

impl ui::menu::Item for DebugTemplate {
type Data = ();

fn format(&self, _data: &Self::Data) -> Row {
fn format(&self, _data: &Self::Data, _theme: Option<&Theme>) -> Row {
self.name.as_str().into()
}
}

impl ui::menu::Item for Thread {
type Data = ThreadStates;

fn format(&self, thread_states: &Self::Data) -> Row {
fn format(&self, thread_states: &Self::Data, _theme: Option<&Theme>) -> Row {
format!(
"{} ({})",
self.name,
Expand Down
12 changes: 6 additions & 6 deletions helix-term/src/commands/lsp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use helix_view::{
document::{DocumentInlayHints, DocumentInlayHintsId, Mode},
editor::Action,
theme::Style,
Document, View,
Document, Theme, View,
};

use crate::{
Expand Down Expand Up @@ -57,7 +57,7 @@ impl ui::menu::Item for lsp::Location {
/// Current working directory.
type Data = PathBuf;

fn format(&self, cwdir: &Self::Data) -> Row {
fn format(&self, cwdir: &Self::Data, _theme: Option<&Theme>) -> Row {
// The preallocation here will overallocate a few characters since it will account for the
// URL's scheme, which is not used most of the time since that scheme will be "file://".
// Those extra chars will be used to avoid allocating when writing the line number (in the
Expand Down Expand Up @@ -91,7 +91,7 @@ impl ui::menu::Item for lsp::SymbolInformation {
/// Path to currently focussed document
type Data = Option<lsp::Url>;

fn format(&self, current_doc_path: &Self::Data) -> Row {
fn format(&self, current_doc_path: &Self::Data, _theme: Option<&Theme>) -> Row {
if current_doc_path.as_ref() == Some(&self.location.uri) {
self.name.as_str().into()
} else {
Expand Down Expand Up @@ -121,7 +121,7 @@ struct PickerDiagnostic {
impl ui::menu::Item for PickerDiagnostic {
type Data = (DiagnosticStyles, DiagnosticsFormat);

fn format(&self, (styles, format): &Self::Data) -> Row {
fn format(&self, (styles, format): &Self::Data, _theme: Option<&Theme>) -> Row {
let mut style = self
.diag
.severity
Expand Down Expand Up @@ -476,7 +476,7 @@ pub fn workspace_diagnostics_picker(cx: &mut Context) {

impl ui::menu::Item for lsp::CodeActionOrCommand {
type Data = ();
fn format(&self, _data: &Self::Data) -> Row {
fn format(&self, _data: &Self::Data, _theme: Option<&Theme>) -> Row {
match self {
lsp::CodeActionOrCommand::CodeAction(action) => action.title.as_str().into(),
lsp::CodeActionOrCommand::Command(command) => command.title.as_str().into(),
Expand Down Expand Up @@ -672,7 +672,7 @@ pub fn code_action(cx: &mut Context) {

impl ui::menu::Item for lsp::Command {
type Data = ();
fn format(&self, _data: &Self::Data) -> Row {
fn format(&self, _data: &Self::Data, _theme: Option<&Theme>) -> Row {
self.title.as_str().into()
}
}
Expand Down
83 changes: 49 additions & 34 deletions helix-term/src/ui/completion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use helix_view::{
document::SavePoint,
editor::CompleteAction,
theme::{Modifier, Style},
ViewId,
Theme, ViewId,
};
use tui::{buffer::Buffer as Surface, text::Span};

Expand Down Expand Up @@ -33,11 +33,57 @@ impl menu::Item for CompletionItem {
.into()
}

fn format(&self, _data: &Self::Data) -> menu::Row {
// fn label(&self, _data: &Self::Data) -> Spans {
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean to remove this commented code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, this slipped through.

// self.label.as_str().into()
// }

fn format(&self, _data: &Self::Data, theme: Option<&Theme>) -> menu::Row {
let (lsp_type_label, style) = match self.kind {
Some(lsp::CompletionItemKind::TEXT) => ("text", Some("ui.text")),
Some(lsp::CompletionItemKind::METHOD) => ("method", Some("function.method")),
Some(lsp::CompletionItemKind::FUNCTION) => ("function", Some("function")),
Some(lsp::CompletionItemKind::CONSTRUCTOR) => ("constructor", Some("constructor")),
Some(lsp::CompletionItemKind::FIELD) => ("field", Some("variable.other.member")),
Some(lsp::CompletionItemKind::VARIABLE) => ("variable", Some("variable")),
Some(lsp::CompletionItemKind::CLASS) => ("class", Some("type")),
Some(lsp::CompletionItemKind::INTERFACE) => ("interface", Some("type")),
Some(lsp::CompletionItemKind::MODULE) => ("module", Some("module")),
Some(lsp::CompletionItemKind::PROPERTY) => ("property", Some("attributes")),
Some(lsp::CompletionItemKind::UNIT) => ("unit", Some("constant")),
Some(lsp::CompletionItemKind::VALUE) => ("value", Some("string")),
Some(lsp::CompletionItemKind::ENUM) => ("enum", Some("type")),
Some(lsp::CompletionItemKind::KEYWORD) => ("keyword", Some("keyword")),
Some(lsp::CompletionItemKind::SNIPPET) => ("snippet", None),
Some(lsp::CompletionItemKind::COLOR) => ("color", None),
Some(lsp::CompletionItemKind::FILE) => ("file", None),
Some(lsp::CompletionItemKind::REFERENCE) => ("reference", None),
Some(lsp::CompletionItemKind::FOLDER) => ("folder", None),
Some(lsp::CompletionItemKind::ENUM_MEMBER) => {
("enum_member", Some("type.enum.variant"))
}
Some(lsp::CompletionItemKind::CONSTANT) => ("constant", Some("constant")),
Some(lsp::CompletionItemKind::STRUCT) => ("struct", Some("type")),
Some(lsp::CompletionItemKind::EVENT) => ("event", None),
Some(lsp::CompletionItemKind::OPERATOR) => ("operator", Some("operator")),
Some(lsp::CompletionItemKind::TYPE_PARAMETER) => {
("type_param", Some("function.parameter"))
}
Some(kind) => unimplemented!("{:?}", kind),
None => ("", None),
};
let mut lsp_type_style = theme
.zip(style)
.map(|(theme, style)| theme.get(style))
.unwrap_or_default()
.remove_modifier(Modifier::all())
.add_modifier(Modifier::ITALIC);
Copy link
Member

@dead10ck dead10ck Apr 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Why are we intentionally removing the modifier from the theme and hardcoding italics? What's the point of pulling in user themes if we're just going to ignore them? I personally can't stand italics in code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are pulling the theme to apply the colors. This is for the completions items, they should have a matching modifier, i.e. no bold but all in italic. Please have a look at the screenshots above.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, in my opinion, we should just let the theme decide how to style the completion items here and not hard code style for the user.

lsp_type_style.bg = None;

let deprecated = self.deprecated.unwrap_or_default()
|| self.tags.as_ref().map_or(false, |tags| {
tags.contains(&lsp::CompletionItemTag::DEPRECATED)
});

menu::Row::new(vec![
menu::Cell::from(Span::styled(
self.label.as_str(),
Expand All @@ -47,38 +93,7 @@ impl menu::Item for CompletionItem {
Style::default()
},
)),
menu::Cell::from(match self.kind {
Some(lsp::CompletionItemKind::TEXT) => "text",
Some(lsp::CompletionItemKind::METHOD) => "method",
Some(lsp::CompletionItemKind::FUNCTION) => "function",
Some(lsp::CompletionItemKind::CONSTRUCTOR) => "constructor",
Some(lsp::CompletionItemKind::FIELD) => "field",
Some(lsp::CompletionItemKind::VARIABLE) => "variable",
Some(lsp::CompletionItemKind::CLASS) => "class",
Some(lsp::CompletionItemKind::INTERFACE) => "interface",
Some(lsp::CompletionItemKind::MODULE) => "module",
Some(lsp::CompletionItemKind::PROPERTY) => "property",
Some(lsp::CompletionItemKind::UNIT) => "unit",
Some(lsp::CompletionItemKind::VALUE) => "value",
Some(lsp::CompletionItemKind::ENUM) => "enum",
Some(lsp::CompletionItemKind::KEYWORD) => "keyword",
Some(lsp::CompletionItemKind::SNIPPET) => "snippet",
Some(lsp::CompletionItemKind::COLOR) => "color",
Some(lsp::CompletionItemKind::FILE) => "file",
Some(lsp::CompletionItemKind::REFERENCE) => "reference",
Some(lsp::CompletionItemKind::FOLDER) => "folder",
Some(lsp::CompletionItemKind::ENUM_MEMBER) => "enum_member",
Some(lsp::CompletionItemKind::CONSTANT) => "constant",
Some(lsp::CompletionItemKind::STRUCT) => "struct",
Some(lsp::CompletionItemKind::EVENT) => "event",
Some(lsp::CompletionItemKind::OPERATOR) => "operator",
Some(lsp::CompletionItemKind::TYPE_PARAMETER) => "type_param",
Some(kind) => {
log::error!("Received unknown completion item kind: {:?}", kind);
""
}
None => "",
}),
menu::Cell::from(lsp_type_label).style(lsp_type_style),
// self.detail.as_deref().unwrap_or("")
// self.label_details
// .as_ref()
Expand Down
16 changes: 8 additions & 8 deletions helix-term/src/ui/menu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,22 @@ pub use tui::widgets::{Cell, Row};
use fuzzy_matcher::skim::SkimMatcherV2 as Matcher;
use fuzzy_matcher::FuzzyMatcher;

use helix_view::{graphics::Rect, Editor};
use helix_view::{graphics::Rect, Editor, Theme};
use tui::layout::Constraint;

pub trait Item {
/// Additional editor state that is used for label calculation.
type Data;

fn format(&self, data: &Self::Data) -> Row;
fn format(&self, data: &Self::Data, _theme: Option<&Theme>) -> Row;

fn sort_text(&self, data: &Self::Data) -> Cow<str> {
let label: String = self.format(data).cell_text().collect();
let label: String = self.format(data, None).cell_text().collect();
label.into()
}

fn filter_text(&self, data: &Self::Data) -> Cow<str> {
let label: String = self.format(data).cell_text().collect();
let label: String = self.format(data, None).cell_text().collect();
label.into()
}
}
Expand All @@ -35,7 +35,7 @@ impl Item for PathBuf {
/// Root prefix to strip.
type Data = PathBuf;

fn format(&self, root_path: &Self::Data) -> Row {
fn format(&self, root_path: &Self::Data, _theme: Option<&Theme>) -> Row {
self.strip_prefix(root_path)
.unwrap_or(self)
.to_string_lossy()
Expand Down Expand Up @@ -142,10 +142,10 @@ impl<T: Item> Menu<T> {
let n = self
.options
.first()
.map(|option| option.format(&self.editor_data).cells.len())
.map(|option| option.format(&self.editor_data, None).cells.len())
.unwrap_or_default();
let max_lens = self.options.iter().fold(vec![0; n], |mut acc, option| {
let row = option.format(&self.editor_data);
let row = option.format(&self.editor_data, None);
// maintain max for each column
for (acc, cell) in acc.iter_mut().zip(row.cells.iter()) {
let width = cell.content.width();
Expand Down Expand Up @@ -331,7 +331,7 @@ impl<T: Item + 'static> Component for Menu<T> {

let rows = options
.iter()
.map(|option| option.format(&self.editor_data));
.map(|option| option.format(&self.editor_data, Some(theme)));
let table = Table::new(rows)
.style(style)
.highlight_style(selected)
Expand Down
6 changes: 3 additions & 3 deletions helix-term/src/ui/picker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -485,10 +485,10 @@ impl<T: Item> Picker<T> {
let n = self
.options
.first()
.map(|option| option.format(&self.editor_data).cells.len())
.map(|option| option.format(&self.editor_data, None).cells.len())
.unwrap_or_default();
let max_lens = self.options.iter().fold(vec![0; n], |mut acc, option| {
let row = option.format(&self.editor_data);
let row = option.format(&self.editor_data, None);
// maintain max for each column
for (acc, cell) in acc.iter_mut().zip(row.cells.iter()) {
let width = cell.content.width();
Expand Down Expand Up @@ -779,7 +779,7 @@ impl<T: Item + 'static> Component for Picker<T> {
.skip(offset)
.take(rows as usize)
.map(|pmatch| &self.options[pmatch.index])
.map(|option| option.format(&self.editor_data))
.map(|option| option.format(&self.editor_data, None))
.map(|mut row| {
const TEMP_CELL_SEP: &str = " ";

Expand Down