From 59dea5a63b6f48abf16e009f43f745291079c61e Mon Sep 17 00:00:00 2001 From: theteachr Date: Fri, 19 Jan 2024 20:15:13 +0530 Subject: [PATCH 1/3] Fix error message shown for goto references When a symbol with no references is gr'd on, the message shown is `No definition found.` Make it `No references found.` Achieves this by introducing empty container checks at call sites rendering one of the patterns redundant. Another possible solution is to pass the error message as another argument. --- helix-term/src/commands/lsp.rs | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 051cdcd35e5d..291d9cdf5a1d 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -1023,8 +1023,9 @@ fn goto_impl( [location] => { jump_to_location(editor, location, offset_encoding, Action::Replace); } + // XXX: All call sites pass non empty containers. This pattern is redundant. [] => { - editor.set_error("No definition found."); + editor.set_error("No results found."); } _locations => { let picker = Picker::new(locations, cwdir, move |cx, location, action| { @@ -1065,9 +1066,11 @@ where cx.callback( future, - move |editor, compositor, response: Option| { - let items = to_locations(response); - goto_impl(editor, compositor, items, offset_encoding); + move |editor, compositor, response: Option| match to_locations( + response, + ) { + items if items.is_empty() => editor.set_error("No definition found."), + items => goto_impl(editor, compositor, items, offset_encoding), }, ); } @@ -1125,9 +1128,11 @@ pub fn goto_reference(cx: &mut Context) { cx.callback( future, - move |editor, compositor, response: Option>| { - let items = response.unwrap_or_default(); - goto_impl(editor, compositor, items, offset_encoding); + move |editor, compositor, response: Option>| match response + .unwrap_or_default() + { + items if items.is_empty() => editor.set_error("No references found."), + items => goto_impl(editor, compositor, items, offset_encoding), }, ); } From 7803fcf49fb1952d874bec6353c9eec84934b0bc Mon Sep 17 00:00:00 2001 From: theteachr Date: Fri, 19 Jan 2024 21:19:35 +0530 Subject: [PATCH 2/3] Use `if-else` over `match` --- helix-term/src/commands/lsp.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 291d9cdf5a1d..db3294c18399 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -1066,11 +1066,13 @@ where cx.callback( future, - move |editor, compositor, response: Option| match to_locations( - response, - ) { - items if items.is_empty() => editor.set_error("No definition found."), - items => goto_impl(editor, compositor, items, offset_encoding), + move |editor, compositor, response: Option| { + let items = to_locations(response); + if items.is_empty() { + editor.set_error("No definition found."); + } else { + goto_impl(editor, compositor, items, offset_encoding); + } }, ); } @@ -1128,11 +1130,13 @@ pub fn goto_reference(cx: &mut Context) { cx.callback( future, - move |editor, compositor, response: Option>| match response - .unwrap_or_default() - { - items if items.is_empty() => editor.set_error("No references found."), - items => goto_impl(editor, compositor, items, offset_encoding), + move |editor, compositor, response: Option>| { + let items = response.unwrap_or_default(); + if items.is_empty() { + editor.set_error("No references found."); + } else { + goto_impl(editor, compositor, items, offset_encoding); + } }, ); } From e6ba42934868f9cef9d55e2a37534e7f1c30fd31 Mon Sep 17 00:00:00 2001 From: theteachr Date: Fri, 19 Jan 2024 21:20:32 +0530 Subject: [PATCH 3/3] Document the precondition for `goto_impl` --- helix-term/src/commands/lsp.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index db3294c18399..bc8559b5dff0 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -1011,6 +1011,7 @@ pub fn apply_workspace_edit( Ok(()) } +/// Precondition: `locations` should be non-empty. fn goto_impl( editor: &mut Editor, compositor: &mut Compositor, @@ -1023,10 +1024,7 @@ fn goto_impl( [location] => { jump_to_location(editor, location, offset_encoding, Action::Replace); } - // XXX: All call sites pass non empty containers. This pattern is redundant. - [] => { - editor.set_error("No results found."); - } + [] => unreachable!("`locations` should be non-empty for `goto_impl`"), _locations => { let picker = Picker::new(locations, cwdir, move |cx, location, action| { jump_to_location(cx.editor, location, offset_encoding, action)