From 3edca7854e66cbdb0c4baca25962a4f390fede55 Mon Sep 17 00:00:00 2001 From: Blaž Hrastnik Date: Mon, 25 Oct 2021 11:03:18 +0900 Subject: completion: fully revert state before apply & insertText common prefix --- helix-term/src/ui/completion.rs | 71 +++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 41 deletions(-) (limited to 'helix-term/src/ui/completion.rs') diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index c75b24f1..44879fcf 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -5,7 +5,7 @@ use tui::buffer::Buffer as Surface; use std::borrow::Cow; use helix_core::Transaction; -use helix_view::{graphics::Rect, Document, Editor, View}; +use helix_view::{graphics::Rect, Document, Editor}; use crate::commands; use crate::ui::{menu, Markdown, Menu, Popup, PromptEvent}; @@ -83,13 +83,13 @@ impl Completion { start_offset: usize, trigger_offset: usize, ) -> Self { - // let items: Vec = Vec::new(); let menu = Menu::new(items, move |editor: &mut Editor, item, event| { fn item_to_transaction( doc: &Document, - view: &View, item: &CompletionItem, offset_encoding: helix_lsp::OffsetEncoding, + start_offset: usize, + trigger_offset: usize, ) -> Transaction { if let Some(edit) = &item.text_edit { let edit = match edit { @@ -105,63 +105,52 @@ impl Completion { ) } else { let text = item.insert_text.as_ref().unwrap_or(&item.label); - let cursor = doc - .selection(view.id) - .primary() - .cursor(doc.text().slice(..)); + // Some LSPs just give you an insertText with no offset ¯\_(ツ)_/¯ + // in these cases we need to check for a common prefix and remove it + let prefix = Cow::from(doc.text().slice(start_offset..trigger_offset)); + let text = text.trim_start_matches::<&str>(&prefix); Transaction::change( doc.text(), - vec![(cursor, cursor, Some(text.as_str().into()))].into_iter(), + vec![(trigger_offset, trigger_offset, Some(text.into()))].into_iter(), ) } } + let (view, doc) = current!(editor); + + // if more text was entered, remove it + doc.restore(view.id); + match event { PromptEvent::Abort => {} PromptEvent::Update => { - let (view, doc) = current!(editor); - // always present here let item = item.unwrap(); - // if more text was entered, remove it - // TODO: ideally to undo we should keep the last completion tx revert, and map it over new changes - let cursor = doc - .selection(view.id) - .primary() - .cursor(doc.text().slice(..)); - if trigger_offset < cursor { - let remove = Transaction::change( - doc.text(), - vec![(trigger_offset, cursor, None)].into_iter(), - ); - doc.apply(&remove, view.id); - } + let transaction = item_to_transaction( + doc, + item, + offset_encoding, + start_offset, + trigger_offset, + ); + + // initialize a savepoint + doc.savepoint(); - let transaction = item_to_transaction(doc, view, item, offset_encoding); doc.apply(&transaction, view.id); } PromptEvent::Validate => { - let (view, doc) = current!(editor); - // always present here let item = item.unwrap(); - // if more text was entered, remove it - // TODO: ideally to undo we should keep the last completion tx revert, and map it over new changes - let cursor = doc - .selection(view.id) - .primary() - .cursor(doc.text().slice(..)); - if trigger_offset < cursor { - let remove = Transaction::change( - doc.text(), - vec![(trigger_offset, cursor, None)].into_iter(), - ); - doc.apply(&remove, view.id); - } - - let transaction = item_to_transaction(doc, view, item, offset_encoding); + let transaction = item_to_transaction( + doc, + item, + offset_encoding, + start_offset, + trigger_offset, + ); doc.apply(&transaction, view.id); if let Some(additional_edits) = &item.additional_text_edits { -- cgit v1.2.3-70-g09d2 From f331ba9df4d7d22a5e9599737da581feb630f748 Mon Sep 17 00:00:00 2001 From: CossonLeo Date: Tue, 26 Oct 2021 08:42:37 +0800 Subject: Clear competion items when start_offset > cursor (#906) --- helix-term/src/ui/completion.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'helix-term/src/ui/completion.rs') diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index 44879fcf..a893e70b 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -199,7 +199,7 @@ impl Completion { .selection(view.id) .primary() .cursor(doc.text().slice(..)); - if self.start_offset <= cursor { + if self.trigger_offset <= cursor { let fragment = doc.text().slice(self.start_offset..cursor); let text = Cow::from(fragment); // TODO: logic is same as ui/picker -- cgit v1.2.3-70-g09d2 From 49f6c2623fbda5ff4be86e5e7d773bf900d9c75c Mon Sep 17 00:00:00 2001 From: Ivan Tham Date: Fri, 29 Oct 2021 11:00:18 +0800 Subject: Bump lsp-types to 0.91.0 (#932) --- Cargo.lock | 4 ++-- helix-lsp/Cargo.toml | 2 +- helix-lsp/src/client.rs | 7 +++--- helix-term/src/application.rs | 9 ++++---- helix-term/src/ui/completion.rs | 51 +++++++++++++++++++++-------------------- 5 files changed, 38 insertions(+), 35 deletions(-) (limited to 'helix-term/src/ui/completion.rs') diff --git a/Cargo.lock b/Cargo.lock index 45a8f5da..8af5c45c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -584,9 +584,9 @@ dependencies = [ [[package]] name = "lsp-types" -version = "0.90.1" +version = "0.91.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6f3734ab1d7d157fc0c45110e06b587c31cd82bea2ccfd6b563cbff0aaeeb1d3" +checksum = "be7801b458592d0998af808d97f6a85a6057af3aaf2a2a5c3c677702bbeb4ed7" dependencies = [ "bitflags", "serde", diff --git a/helix-lsp/Cargo.toml b/helix-lsp/Cargo.toml index f9910cc0..8cbff41d 100644 --- a/helix-lsp/Cargo.toml +++ b/helix-lsp/Cargo.toml @@ -19,7 +19,7 @@ futures-executor = "0.3" futures-util = { version = "0.3", features = ["std", "async-await"], default-features = false } jsonrpc-core = { version = "18.0", default-features = false } # don't pull in all of futures log = "0.4" -lsp-types = { version = "0.90", features = ["proposed"] } +lsp-types = { version = "0.91", features = ["proposed"] } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" thiserror = "1.0" diff --git a/helix-lsp/src/client.rs b/helix-lsp/src/client.rs index 4068ae1f..b810feef 100644 --- a/helix-lsp/src/client.rs +++ b/helix-lsp/src/client.rs @@ -461,7 +461,7 @@ impl Client { }; let changes = match sync_capabilities { - lsp::TextDocumentSyncKind::Full => { + lsp::TextDocumentSyncKind::FULL => { vec![lsp::TextDocumentContentChangeEvent { // range = None -> whole document range: None, //Some(Range) @@ -469,10 +469,11 @@ impl Client { text: new_text.to_string(), }] } - lsp::TextDocumentSyncKind::Incremental => { + lsp::TextDocumentSyncKind::INCREMENTAL => { Self::changeset_to_changes(old_text, new_text, changes, self.offset_encoding) } - lsp::TextDocumentSyncKind::None => return None, + lsp::TextDocumentSyncKind::NONE => return None, + kind => unimplemented!("{:?}", kind), }; Some(self.notify::( diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 55b12c5a..6037148f 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -389,10 +389,11 @@ impl Application { message: diagnostic.message, severity: diagnostic.severity.map( |severity| match severity { - DiagnosticSeverity::Error => Error, - DiagnosticSeverity::Warning => Warning, - DiagnosticSeverity::Information => Info, - DiagnosticSeverity::Hint => Hint, + DiagnosticSeverity::ERROR => Error, + DiagnosticSeverity::WARNING => Warning, + DiagnosticSeverity::INFORMATION => Info, + DiagnosticSeverity::HINT => Hint, + severity => unimplemented!("{:?}", severity), }, ), // code diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index a893e70b..dcb2bfd8 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -30,31 +30,32 @@ impl menu::Item for CompletionItem { menu::Row::new(vec![ menu::Cell::from(self.label.as_str()), 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::EnumMember) => "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::TypeParameter) => "type_param", + 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) => unimplemented!("{:?}", kind), None => "", }), // self.detail.as_deref().unwrap_or("") -- cgit v1.2.3-70-g09d2 From 3eb829e2330fed5ad1c095f8bba44f62361b4943 Mon Sep 17 00:00:00 2001 From: Ivan Tham Date: Wed, 3 Nov 2021 11:02:29 +0800 Subject: Ensure coords in screen depends on char width (#885) The issue affected files with lots of tabs at the start as well. Fix #840--- helix-core/src/lib.rs | 2 +- helix-core/src/movement.rs | 4 ++ helix-core/src/position.rs | 81 +++++++++++++++++++++++++++++++++++++---- helix-term/src/ui/completion.rs | 10 ++--- helix-view/src/view.rs | 8 ++-- 5 files changed, 87 insertions(+), 18 deletions(-) (limited to 'helix-term/src/ui/completion.rs') diff --git a/helix-core/src/lib.rs b/helix-core/src/lib.rs index 96f88ee4..d1720df0 100644 --- a/helix-core/src/lib.rs +++ b/helix-core/src/lib.rs @@ -194,7 +194,7 @@ pub use tendril::StrTendril as Tendril; pub use {regex, tree_sitter}; pub use graphemes::RopeGraphemes; -pub use position::{coords_at_pos, pos_at_coords, Position}; +pub use position::{coords_at_pos, pos_at_coords, visual_coords_at_pos, Position}; pub use selection::{Range, Selection}; pub use smallvec::SmallVec; pub use syntax::Syntax; diff --git a/helix-core/src/movement.rs b/helix-core/src/movement.rs index 5d080545..9e85bd21 100644 --- a/helix-core/src/movement.rs +++ b/helix-core/src/movement.rs @@ -53,6 +53,10 @@ pub fn move_vertically( let pos = range.cursor(slice); // Compute the current position's 2d coordinates. + // TODO: switch this to use `visual_coords_at_pos` rather than + // `coords_at_pos` as this will cause a jerky movement when the visual + // position does not match, like moving from a line with tabs/CJK to + // a line without let Position { row, col } = coords_at_pos(slice, pos); let horiz = range.horiz.unwrap_or(col as u32); diff --git a/helix-core/src/position.rs b/helix-core/src/position.rs index 08a8aed5..c6018ce6 100644 --- a/helix-core/src/position.rs +++ b/helix-core/src/position.rs @@ -2,6 +2,7 @@ use crate::{ chars::char_is_line_ending, graphemes::{ensure_grapheme_boundary_prev, RopeGraphemes}, line_ending::line_end_char_index, + unicode::width::UnicodeWidthChar, RopeSlice, }; @@ -54,11 +55,8 @@ impl From for tree_sitter::Point { } /// Convert a character index to (line, column) coordinates. /// -/// TODO: this should be split into two methods: one for visual -/// row/column, and one for "objective" row/column (possibly with -/// the column specified in `char`s). The former would be used -/// for cursor movement, and the latter would be used for e.g. the -/// row:column display in the status line. +/// column in `char` count which can be used for row:column display in +/// status line. See [`visual_coords_at_pos`] for a visual one. pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position { let line = text.char_to_line(pos); @@ -69,6 +67,28 @@ pub fn coords_at_pos(text: RopeSlice, pos: usize) -> Position { Position::new(line, col) } +/// Convert a character index to (line, column) coordinates visually. +/// +/// Takes \t, double-width characters (CJK) into account as well as text +/// not in the document in the future. +/// See [`coords_at_pos`] for an "objective" one. +pub fn visual_coords_at_pos(text: RopeSlice, pos: usize, tab_width: usize) -> Position { + let line = text.char_to_line(pos); + + let line_start = text.line_to_char(line); + let pos = ensure_grapheme_boundary_prev(text, pos); + let col = text + .slice(line_start..pos) + .chars() + .flat_map(|c| match c { + '\t' => Some(tab_width), + c => UnicodeWidthChar::width(c), + }) + .sum(); + + Position::new(line, col) +} + /// Convert (line, column) coordinates to a character index. /// /// If the `line` coordinate is beyond the end of the file, the EOF @@ -130,7 +150,6 @@ mod test { assert_eq!(coords_at_pos(slice, 10), (1, 4).into()); // position on d // Test with wide characters. - // TODO: account for character width. let text = Rope::from("今日はいい\n"); let slice = text.slice(..); assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); @@ -151,7 +170,6 @@ mod test { assert_eq!(coords_at_pos(slice, 9), (1, 0).into()); // Test with wide-character grapheme clusters. - // TODO: account for character width. let text = Rope::from("किमपि\n"); let slice = text.slice(..); assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); @@ -161,7 +179,6 @@ mod test { assert_eq!(coords_at_pos(slice, 6), (1, 0).into()); // Test with tabs. - // Todo: account for tab stops. let text = Rope::from("\tHello\n"); let slice = text.slice(..); assert_eq!(coords_at_pos(slice, 0), (0, 0).into()); @@ -169,6 +186,54 @@ mod test { assert_eq!(coords_at_pos(slice, 2), (0, 2).into()); } + #[test] + fn test_visual_coords_at_pos() { + let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ"); + let slice = text.slice(..); + assert_eq!(visual_coords_at_pos(slice, 0, 8), (0, 0).into()); + assert_eq!(visual_coords_at_pos(slice, 5, 8), (0, 5).into()); // position on \n + assert_eq!(visual_coords_at_pos(slice, 6, 8), (1, 0).into()); // position on w + assert_eq!(visual_coords_at_pos(slice, 7, 8), (1, 1).into()); // position on o + assert_eq!(visual_coords_at_pos(slice, 10, 8), (1, 4).into()); // position on d + + // Test with wide characters. + let text = Rope::from("今日はいい\n"); + let slice = text.slice(..); + assert_eq!(visual_coords_at_pos(slice, 0, 8), (0, 0).into()); + assert_eq!(visual_coords_at_pos(slice, 1, 8), (0, 2).into()); + assert_eq!(visual_coords_at_pos(slice, 2, 8), (0, 4).into()); + assert_eq!(visual_coords_at_pos(slice, 3, 8), (0, 6).into()); + assert_eq!(visual_coords_at_pos(slice, 4, 8), (0, 8).into()); + assert_eq!(visual_coords_at_pos(slice, 5, 8), (0, 10).into()); + assert_eq!(visual_coords_at_pos(slice, 6, 8), (1, 0).into()); + + // Test with grapheme clusters. + let text = Rope::from("a̐éö̲\r\n"); + let slice = text.slice(..); + assert_eq!(visual_coords_at_pos(slice, 0, 8), (0, 0).into()); + assert_eq!(visual_coords_at_pos(slice, 2, 8), (0, 1).into()); + assert_eq!(visual_coords_at_pos(slice, 4, 8), (0, 2).into()); + assert_eq!(visual_coords_at_pos(slice, 7, 8), (0, 3).into()); + assert_eq!(visual_coords_at_pos(slice, 9, 8), (1, 0).into()); + + // Test with wide-character grapheme clusters. + // TODO: account for cluster. + let text = Rope::from("किमपि\n"); + let slice = text.slice(..); + assert_eq!(visual_coords_at_pos(slice, 0, 8), (0, 0).into()); + assert_eq!(visual_coords_at_pos(slice, 2, 8), (0, 2).into()); + assert_eq!(visual_coords_at_pos(slice, 3, 8), (0, 3).into()); + assert_eq!(visual_coords_at_pos(slice, 5, 8), (0, 5).into()); + assert_eq!(visual_coords_at_pos(slice, 6, 8), (1, 0).into()); + + // Test with tabs. + let text = Rope::from("\tHello\n"); + let slice = text.slice(..); + assert_eq!(visual_coords_at_pos(slice, 0, 8), (0, 0).into()); + assert_eq!(visual_coords_at_pos(slice, 1, 8), (0, 8).into()); + assert_eq!(visual_coords_at_pos(slice, 2, 8), (0, 9).into()); + } + #[test] fn test_pos_at_coords() { let text = Rope::from("ḧëḷḷö\nẅöṛḷḋ"); diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs index dcb2bfd8..dd782d29 100644 --- a/helix-term/src/ui/completion.rs +++ b/helix-term/src/ui/completion.rs @@ -264,12 +264,10 @@ impl Component for Completion { .language() .and_then(|scope| scope.strip_prefix("source.")) .unwrap_or(""); - let cursor_pos = doc - .selection(view.id) - .primary() - .cursor(doc.text().slice(..)); - let cursor_pos = (helix_core::coords_at_pos(doc.text().slice(..), cursor_pos).row - - view.offset.row) as u16; + let text = doc.text().slice(..); + let cursor_pos = doc.selection(view.id).primary().cursor(text); + let coords = helix_core::visual_coords_at_pos(text, cursor_pos, doc.tab_width()); + let cursor_pos = (coords.row - view.offset.row) as u16; let mut markdown_doc = match &option.documentation { Some(lsp::Documentation::String(contents)) | Some(lsp::Documentation::MarkupContent(lsp::MarkupContent { diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index 01f18c71..11f30155 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -2,10 +2,9 @@ use std::borrow::Cow; use crate::{graphics::Rect, Document, DocumentId, ViewId}; use helix_core::{ - coords_at_pos, graphemes::{grapheme_width, RopeGraphemes}, line_ending::line_end_char_index, - Position, RopeSlice, Selection, + visual_coords_at_pos, Position, RopeSlice, Selection, }; type Jump = (DocumentId, Selection); @@ -91,7 +90,10 @@ impl View { .selection(self.id) .primary() .cursor(doc.text().slice(..)); - let Position { col, row: line } = coords_at_pos(doc.text().slice(..), cursor); + + let Position { col, row: line } = + visual_coords_at_pos(doc.text().slice(..), cursor, doc.tab_width()); + let inner_area = self.inner_area(); let last_line = (self.offset.row + inner_area.height as usize).saturating_sub(1); -- cgit v1.2.3-70-g09d2