diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index dfd526b7e8..594586f115 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -5368,7 +5368,6 @@ impl Editor { mat.candidate_id }; - let buffer_handle = completions_menu.buffer; let completion = completions_menu .completions .borrow() @@ -5376,34 +5375,23 @@ impl Editor { .clone(); cx.stop_propagation(); + let buffer_handle = completions_menu.buffer; + + let CompletionEdit { + new_text, + snippet, + replace_range, + } = process_completion_for_edit( + &completion, + intent, + &buffer_handle, + &completions_menu.initial_position.text_anchor, + cx, + ); + + let buffer = buffer_handle.read(cx); let snapshot = self.buffer.read(cx).snapshot(cx); let newest_anchor = self.selections.newest_anchor(); - - let snippet; - let new_text; - if completion.is_snippet() { - let mut snippet_source = completion.new_text.clone(); - if let Some(scope) = snapshot.language_scope_at(newest_anchor.head()) { - if scope.prefers_label_for_snippet_in_completion() { - if let Some(label) = completion.label() { - if matches!( - completion.kind(), - Some(CompletionItemKind::FUNCTION) | Some(CompletionItemKind::METHOD) - ) { - snippet_source = label; - } - } - } - } - snippet = Some(Snippet::parse(&snippet_source).log_err()?); - new_text = snippet.as_ref().unwrap().text.clone(); - } else { - snippet = None; - new_text = completion.new_text.clone(); - }; - - let replace_range = choose_completion_range(&completion, intent, &buffer_handle, cx); - let buffer = buffer_handle.read(cx); let replace_range_multibuffer = { let excerpt = snapshot.excerpt_containing(newest_anchor.range()).unwrap(); let multibuffer_anchor = snapshot @@ -19514,79 +19502,152 @@ fn vim_enabled(cx: &App) -> bool { == Some(&serde_json::Value::Bool(true)) } -// Consider user intent and default settings -fn choose_completion_range( +fn process_completion_for_edit( completion: &Completion, intent: CompletionIntent, buffer: &Entity, + cursor_position: &text::Anchor, cx: &mut Context, -) -> Range { - fn should_replace( - completion: &Completion, - insert_range: &Range, - intent: CompletionIntent, - completion_mode_setting: LspInsertMode, - buffer: &Buffer, - ) -> bool { - // specific actions take precedence over settings - match intent { - CompletionIntent::CompleteWithInsert => return false, - CompletionIntent::CompleteWithReplace => return true, - CompletionIntent::Complete | CompletionIntent::Compose => {} - } - - match completion_mode_setting { - LspInsertMode::Insert => false, - LspInsertMode::Replace => true, - LspInsertMode::ReplaceSubsequence => { - let mut text_to_replace = buffer.chars_for_range( - buffer.anchor_before(completion.replace_range.start) - ..buffer.anchor_after(completion.replace_range.end), - ); - let mut completion_text = completion.new_text.chars(); - - // is `text_to_replace` a subsequence of `completion_text` - text_to_replace - .all(|needle_ch| completion_text.any(|haystack_ch| haystack_ch == needle_ch)) - } - LspInsertMode::ReplaceSuffix => { - let range_after_cursor = insert_range.end..completion.replace_range.end; - - let text_after_cursor = buffer - .text_for_range( - buffer.anchor_before(range_after_cursor.start) - ..buffer.anchor_after(range_after_cursor.end), - ) - .collect::(); - completion.new_text.ends_with(&text_after_cursor) - } - } - } - +) -> CompletionEdit { let buffer = buffer.read(cx); - - if let CompletionSource::Lsp { - insert_range: Some(insert_range), - .. - } = &completion.source - { - let completion_mode_setting = - language_settings(buffer.language().map(|l| l.name()), buffer.file(), cx) - .completions - .lsp_insert_mode; - - if !should_replace( - completion, - &insert_range, - intent, - completion_mode_setting, - buffer, - ) { - return insert_range.to_offset(buffer); + let buffer_snapshot = buffer.snapshot(); + let (snippet, new_text) = if completion.is_snippet() { + let mut snippet_source = completion.new_text.clone(); + if let Some(scope) = buffer_snapshot.language_scope_at(cursor_position) { + if scope.prefers_label_for_snippet_in_completion() { + if let Some(label) = completion.label() { + if matches!( + completion.kind(), + Some(CompletionItemKind::FUNCTION) | Some(CompletionItemKind::METHOD) + ) { + snippet_source = label; + } + } + } } + match Snippet::parse(&snippet_source).log_err() { + Some(parsed_snippet) => (Some(parsed_snippet.clone()), parsed_snippet.text), + None => (None, completion.new_text.clone()), + } + } else { + (None, completion.new_text.clone()) + }; + + let mut range_to_replace = { + let replace_range = &completion.replace_range; + if let CompletionSource::Lsp { + insert_range: Some(insert_range), + .. + } = &completion.source + { + debug_assert_eq!( + insert_range.start, replace_range.start, + "insert_range and replace_range should start at the same position" + ); + debug_assert!( + insert_range + .start + .cmp(&cursor_position, &buffer_snapshot) + .is_le(), + "insert_range should start before or at cursor position" + ); + debug_assert!( + replace_range + .start + .cmp(&cursor_position, &buffer_snapshot) + .is_le(), + "replace_range should start before or at cursor position" + ); + debug_assert!( + insert_range + .end + .cmp(&cursor_position, &buffer_snapshot) + .is_le(), + "insert_range should end before or at cursor position" + ); + + let should_replace = match intent { + CompletionIntent::CompleteWithInsert => false, + CompletionIntent::CompleteWithReplace => true, + CompletionIntent::Complete | CompletionIntent::Compose => { + let insert_mode = + language_settings(buffer.language().map(|l| l.name()), buffer.file(), cx) + .completions + .lsp_insert_mode; + match insert_mode { + LspInsertMode::Insert => false, + LspInsertMode::Replace => true, + LspInsertMode::ReplaceSubsequence => { + let mut text_to_replace = buffer.chars_for_range( + buffer.anchor_before(replace_range.start) + ..buffer.anchor_after(replace_range.end), + ); + let mut current_needle = text_to_replace.next(); + for haystack_ch in completion.label.text.chars() { + if let Some(needle_ch) = current_needle { + if haystack_ch.eq_ignore_ascii_case(&needle_ch) { + current_needle = text_to_replace.next(); + } + } + } + current_needle.is_none() + } + LspInsertMode::ReplaceSuffix => { + if replace_range + .end + .cmp(&cursor_position, &buffer_snapshot) + .is_gt() + { + let range_after_cursor = *cursor_position..replace_range.end; + let text_after_cursor = buffer + .text_for_range( + buffer.anchor_before(range_after_cursor.start) + ..buffer.anchor_after(range_after_cursor.end), + ) + .collect::() + .to_ascii_lowercase(); + completion + .label + .text + .to_ascii_lowercase() + .ends_with(&text_after_cursor) + } else { + true + } + } + } + } + }; + + if should_replace { + replace_range.clone() + } else { + insert_range.clone() + } + } else { + replace_range.clone() + } + }; + + if range_to_replace + .end + .cmp(&cursor_position, &buffer_snapshot) + .is_lt() + { + range_to_replace.end = *cursor_position; } - completion.replace_range.to_offset(buffer) + CompletionEdit { + new_text, + replace_range: range_to_replace.to_offset(&buffer), + snippet, + } +} + +struct CompletionEdit { + new_text: String, + replace_range: Range, + snippet: Option, } fn insert_extra_newline_brackets( diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 96516f6b30..f2937591a3 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -10479,6 +10479,7 @@ async fn test_completion_mode(cx: &mut TestAppContext) { run_description: &'static str, initial_state: String, buffer_marked_text: String, + completion_label: &'static str, completion_text: &'static str, expected_with_insert_mode: String, expected_with_replace_mode: String, @@ -10491,6 +10492,7 @@ async fn test_completion_mode(cx: &mut TestAppContext) { run_description: "Start of word matches completion text", initial_state: "before ediˇ after".into(), buffer_marked_text: "before after".into(), + completion_label: "editor", completion_text: "editor", expected_with_insert_mode: "before editorˇ after".into(), expected_with_replace_mode: "before editorˇ after".into(), @@ -10501,6 +10503,7 @@ async fn test_completion_mode(cx: &mut TestAppContext) { run_description: "Accept same text at the middle of the word", initial_state: "before ediˇtor after".into(), buffer_marked_text: "before after".into(), + completion_label: "editor", completion_text: "editor", expected_with_insert_mode: "before editorˇtor after".into(), expected_with_replace_mode: "before editorˇ after".into(), @@ -10511,6 +10514,7 @@ async fn test_completion_mode(cx: &mut TestAppContext) { run_description: "End of word matches completion text -- cursor at end", initial_state: "before torˇ after".into(), buffer_marked_text: "before after".into(), + completion_label: "editor", completion_text: "editor", expected_with_insert_mode: "before editorˇ after".into(), expected_with_replace_mode: "before editorˇ after".into(), @@ -10521,6 +10525,7 @@ async fn test_completion_mode(cx: &mut TestAppContext) { run_description: "End of word matches completion text -- cursor at start", initial_state: "before ˇtor after".into(), buffer_marked_text: "before <|tor> after".into(), + completion_label: "editor", completion_text: "editor", expected_with_insert_mode: "before editorˇtor after".into(), expected_with_replace_mode: "before editorˇ after".into(), @@ -10531,6 +10536,7 @@ async fn test_completion_mode(cx: &mut TestAppContext) { run_description: "Prepend text containing whitespace", initial_state: "pˇfield: bool".into(), buffer_marked_text: ": bool".into(), + completion_label: "pub ", completion_text: "pub ", expected_with_insert_mode: "pub ˇfield: bool".into(), expected_with_replace_mode: "pub ˇ: bool".into(), @@ -10541,6 +10547,7 @@ async fn test_completion_mode(cx: &mut TestAppContext) { run_description: "Add element to start of list", initial_state: "[element_ˇelement_2]".into(), buffer_marked_text: "[]".into(), + completion_label: "element_1", completion_text: "element_1", expected_with_insert_mode: "[element_1ˇelement_2]".into(), expected_with_replace_mode: "[element_1ˇ]".into(), @@ -10551,6 +10558,7 @@ async fn test_completion_mode(cx: &mut TestAppContext) { run_description: "Add element to start of list -- first and second elements are equal", initial_state: "[elˇelement]".into(), buffer_marked_text: "[]".into(), + completion_label: "element", completion_text: "element", expected_with_insert_mode: "[elementˇelement]".into(), expected_with_replace_mode: "[elementˇ]".into(), @@ -10561,6 +10569,7 @@ async fn test_completion_mode(cx: &mut TestAppContext) { run_description: "Ends with matching suffix", initial_state: "SubˇError".into(), buffer_marked_text: "".into(), + completion_label: "SubscriptionError", completion_text: "SubscriptionError", expected_with_insert_mode: "SubscriptionErrorˇError".into(), expected_with_replace_mode: "SubscriptionErrorˇ".into(), @@ -10571,6 +10580,7 @@ async fn test_completion_mode(cx: &mut TestAppContext) { run_description: "Suffix is a subsequence -- contiguous", initial_state: "SubˇErr".into(), buffer_marked_text: "".into(), + completion_label: "SubscriptionError", completion_text: "SubscriptionError", expected_with_insert_mode: "SubscriptionErrorˇErr".into(), expected_with_replace_mode: "SubscriptionErrorˇ".into(), @@ -10581,6 +10591,7 @@ async fn test_completion_mode(cx: &mut TestAppContext) { run_description: "Suffix is a subsequence -- non-contiguous -- replace intended", initial_state: "Suˇscrirr".into(), buffer_marked_text: "".into(), + completion_label: "SubscriptionError", completion_text: "SubscriptionError", expected_with_insert_mode: "SubscriptionErrorˇscrirr".into(), expected_with_replace_mode: "SubscriptionErrorˇ".into(), @@ -10591,12 +10602,46 @@ async fn test_completion_mode(cx: &mut TestAppContext) { run_description: "Suffix is a subsequence -- non-contiguous -- replace unintended", initial_state: "foo(indˇix)".into(), buffer_marked_text: "foo()".into(), + completion_label: "node_index", completion_text: "node_index", expected_with_insert_mode: "foo(node_indexˇix)".into(), expected_with_replace_mode: "foo(node_indexˇ)".into(), expected_with_replace_subsequence_mode: "foo(node_indexˇix)".into(), expected_with_replace_suffix_mode: "foo(node_indexˇix)".into(), }, + Run { + run_description: "Replace range ends before cursor - should extend to cursor", + initial_state: "before editˇo after".into(), + buffer_marked_text: "before <{ed}>it|o after".into(), + completion_label: "editor", + completion_text: "editor", + expected_with_insert_mode: "before editorˇo after".into(), + expected_with_replace_mode: "before editorˇo after".into(), + expected_with_replace_subsequence_mode: "before editorˇo after".into(), + expected_with_replace_suffix_mode: "before editorˇo after".into(), + }, + Run { + run_description: "Uses label for suffix matching", + initial_state: "before ediˇtor after".into(), + buffer_marked_text: "before after".into(), + completion_label: "editor", + completion_text: "editor()", + expected_with_insert_mode: "before editor()ˇtor after".into(), + expected_with_replace_mode: "before editor()ˇ after".into(), + expected_with_replace_subsequence_mode: "before editor()ˇ after".into(), + expected_with_replace_suffix_mode: "before editor()ˇ after".into(), + }, + Run { + run_description: "Case insensitive subsequence and suffix matching", + initial_state: "before EDiˇtoR after".into(), + buffer_marked_text: "before after".into(), + completion_label: "editor", + completion_text: "editor", + expected_with_insert_mode: "before editorˇtoR after".into(), + expected_with_replace_mode: "before editorˇ after".into(), + expected_with_replace_subsequence_mode: "before editorˇ after".into(), + expected_with_replace_suffix_mode: "before editorˇ after".into(), + }, ]; for run in runs { @@ -10637,7 +10682,7 @@ async fn test_completion_mode(cx: &mut TestAppContext) { handle_completion_request_with_insert_and_replace( &mut cx, &run.buffer_marked_text, - vec![run.completion_text], + vec![(run.completion_label, run.completion_text)], counter.clone(), ) .await; @@ -10697,7 +10742,7 @@ async fn test_completion_with_mode_specified_by_action(cx: &mut TestAppContext) handle_completion_request_with_insert_and_replace( &mut cx, &buffer_marked_text, - vec![completion_text], + vec![(completion_text, completion_text)], counter.clone(), ) .await; @@ -10731,7 +10776,7 @@ async fn test_completion_with_mode_specified_by_action(cx: &mut TestAppContext) handle_completion_request_with_insert_and_replace( &mut cx, &buffer_marked_text, - vec![completion_text], + vec![(completion_text, completion_text)], counter.clone(), ) .await; @@ -10818,7 +10863,7 @@ async fn test_completion_replacing_surrounding_text_with_multicursors(cx: &mut T handle_completion_request_with_insert_and_replace( &mut cx, completion_marked_buffer, - vec![completion_text], + vec![(completion_text, completion_text)], Arc::new(AtomicUsize::new(0)), ) .await; @@ -10872,7 +10917,7 @@ async fn test_completion_replacing_surrounding_text_with_multicursors(cx: &mut T handle_completion_request_with_insert_and_replace( &mut cx, completion_marked_buffer, - vec![completion_text], + vec![(completion_text, completion_text)], Arc::new(AtomicUsize::new(0)), ) .await; @@ -10921,7 +10966,7 @@ async fn test_completion_replacing_surrounding_text_with_multicursors(cx: &mut T handle_completion_request_with_insert_and_replace( &mut cx, completion_marked_buffer, - vec![completion_text], + vec![(completion_text, completion_text)], Arc::new(AtomicUsize::new(0)), ) .await; @@ -21064,19 +21109,27 @@ pub fn handle_completion_request( /// Similar to `handle_completion_request`, but a [`CompletionTextEdit::InsertAndReplace`] will be /// given instead, which also contains an `insert` range. /// -/// This function uses the cursor position to mimic what Rust-Analyzer provides as the `insert` range, -/// that is, `replace_range.start..cursor_pos`. +/// This function uses markers to define ranges: +/// - `|` marks the cursor position +/// - `<>` marks the replace range +/// - `[]` marks the insert range (optional, defaults to `replace_range.start..cursor_pos`which is what Rust-Analyzer provides) pub fn handle_completion_request_with_insert_and_replace( cx: &mut EditorLspTestContext, marked_string: &str, - completions: Vec<&'static str>, + completions: Vec<(&'static str, &'static str)>, // (label, new_text) counter: Arc, ) -> impl Future { let complete_from_marker: TextRangeMarker = '|'.into(); let replace_range_marker: TextRangeMarker = ('<', '>').into(); + let insert_range_marker: TextRangeMarker = ('{', '}').into(); + let (_, mut marked_ranges) = marked_text_ranges_by( marked_string, - vec![complete_from_marker.clone(), replace_range_marker.clone()], + vec![ + complete_from_marker.clone(), + replace_range_marker.clone(), + insert_range_marker.clone(), + ], ); let complete_from_position = @@ -21084,6 +21137,14 @@ pub fn handle_completion_request_with_insert_and_replace( let replace_range = cx.to_lsp_range(marked_ranges.remove(&replace_range_marker).unwrap()[0].clone()); + let insert_range = match marked_ranges.remove(&insert_range_marker) { + Some(ranges) if !ranges.is_empty() => cx.to_lsp_range(ranges[0].clone()), + _ => lsp::Range { + start: replace_range.start, + end: complete_from_position, + }, + }; + let mut request = cx.set_request_handler::(move |url, params, _| { let completions = completions.clone(); @@ -21097,16 +21158,13 @@ pub fn handle_completion_request_with_insert_and_replace( Ok(Some(lsp::CompletionResponse::Array( completions .iter() - .map(|completion_text| lsp::CompletionItem { - label: completion_text.to_string(), + .map(|(label, new_text)| lsp::CompletionItem { + label: label.to_string(), text_edit: Some(lsp::CompletionTextEdit::InsertAndReplace( lsp::InsertReplaceEdit { - insert: lsp::Range { - start: replace_range.start, - end: complete_from_position, - }, + insert: insert_range, replace: replace_range, - new_text: completion_text.to_string(), + new_text: new_text.to_string(), }, )), ..Default::default()