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

Avoid saving to jumplist when goto is cancelled #2663

Closed
yzwduck opened this issue Jun 3, 2022 · 1 comment · Fixed by #2670
Closed

Avoid saving to jumplist when goto is cancelled #2663

yzwduck opened this issue Jun 3, 2022 · 1 comment · Fixed by #2670
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements

Comments

@yzwduck
Copy link
Contributor

yzwduck commented Jun 3, 2022

When invoking goto_reference, the current selection will always be saved to the jumplist, even if the goto command is cancelled by pressing Esc in the file picker.

When a goto command is cancelled, no effect is expected (except for querying LSP). But in current implementation, the jumplist will be tampered.

I think this issue can be fixed by the following patch, but I'm new to Rust and not sure if there are other use cases I haven't thought of.

diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs
index bfedb209..8f139b3b 100644
--- a/helix-term/src/commands/lsp.rs
+++ b/helix-term/src/commands/lsp.rs
@@ -485,8 +485,6 @@ fn goto_impl(
     locations: Vec<lsp::Location>,
     offset_encoding: OffsetEncoding,
 ) {
-    push_jump(editor);
-
     let cwdir = std::env::current_dir().expect("couldn't determine current directory");

     match locations.as_slice() {
@@ -520,6 +518,7 @@ fn goto_impl(
                     format!("{}:{}", file, line).into()
                 },
                 move |cx, location, action| {
+                    push_jump(cx.editor);
                     jump_to_location(cx.editor, location, offset_encoding, action)
                 },
                 move |_editor, location| Some(location_to_file_location(location)),
@the-mikedavis the-mikedavis added C-enhancement Category: Improvements A-helix-term Area: Helix term improvements labels Jun 3, 2022
@the-mikedavis
Copy link
Member

Not modifying the jump list until jumping to a reference seems like better behavior to me. That change looks good - I just gave it a try and it works great! Would you like to submit a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-enhancement Category: Improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants