From aa4d84a0b30fbe93882508c6d9dc8ca4c95341d0 Mon Sep 17 00:00:00 2001
From: woojiq
Date: Tue, 8 Aug 2023 16:17:29 +0300
Subject: Align view for background buffer  opened with `alt-ret` (#7691)

* fix(picker): `alt-ret' changes cursor pos of current file, not new one

Closes #7673

* fix other pickers

* symbol pickers
* diagnostick pickers

This is done using the already patched `jump_to_location` method.

* fix global and jumplist pickers

* use `view` as old_id; make `align_view` method of `Action`

* test(picker): basic <alt-ret> functionality

* fix: picker integrational test

* fix nit

Co-authored-by: Michael Davis <mcarsondavis@gmail.com>

---------

Co-authored-by: Michael Davis <mcarsondavis@gmail.com>---
 helix-term/src/commands/lsp.rs | 77 ++++++++++++------------------------------
 1 file changed, 22 insertions(+), 55 deletions(-)

(limited to 'helix-term/src/commands/lsp.rs')

diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs
index 17f936cd..57be1267 100644
--- a/helix-term/src/commands/lsp.rs
+++ b/helix-term/src/commands/lsp.rs
@@ -195,7 +195,6 @@ fn location_to_file_location(location: &lsp::Location) -> FileLocation {
     (path.into(), line)
 }
 
-// TODO: share with symbol picker(symbol.location)
 fn jump_to_location(
     editor: &mut Editor,
     location: &lsp::Location,
@@ -213,15 +212,16 @@ fn jump_to_location(
             return;
         }
     };
-    match editor.open(&path, action) {
-        Ok(_) => (),
+
+    let doc = match editor.open(&path, action) {
+        Ok(id) => doc_mut!(editor, &id),
         Err(err) => {
             let err = format!("failed to open path: {:?}: {:?}", location.uri, err);
             editor.set_error(err);
             return;
         }
-    }
-    let (view, doc) = current!(editor);
+    };
+    let view = view_mut!(editor);
     // TODO: convert inside server
     let new_range =
         if let Some(new_range) = lsp_range_to_range(doc.text(), location.range, offset_encoding) {
@@ -233,45 +233,22 @@ fn jump_to_location(
     // we flip the range so that the cursor sits on the start of the symbol
     // (for example start of the function).
     doc.set_selection(view.id, Selection::single(new_range.head, new_range.anchor));
-    align_view(doc, view, Align::Center);
+    if action.align_view(view, doc.id()) {
+        align_view(doc, view, Align::Center);
+    }
 }
 
 type SymbolPicker = Picker<SymbolInformationItem>;
 
 fn sym_picker(symbols: Vec<SymbolInformationItem>, current_path: Option<lsp::Url>) -> SymbolPicker {
     // TODO: drop current_path comparison and instead use workspace: bool flag?
-    Picker::new(symbols, current_path.clone(), move |cx, item, action| {
-        let (view, doc) = current!(cx.editor);
-        push_jump(view, doc);
-
-        if current_path.as_ref() != Some(&item.symbol.location.uri) {
-            let uri = &item.symbol.location.uri;
-            let path = match uri.to_file_path() {
-                Ok(path) => path,
-                Err(_) => {
-                    let err = format!("unable to convert URI to filepath: {}", uri);
-                    cx.editor.set_error(err);
-                    return;
-                }
-            };
-            if let Err(err) = cx.editor.open(&path, action) {
-                let err = format!("failed to open document: {}: {}", uri, err);
-                log::error!("{}", err);
-                cx.editor.set_error(err);
-                return;
-            }
-        }
-
-        let (view, doc) = current!(cx.editor);
-
-        if let Some(range) =
-            lsp_range_to_range(doc.text(), item.symbol.location.range, item.offset_encoding)
-        {
-            // we flip the range so that the cursor sits on the start of the symbol
-            // (for example start of the function).
-            doc.set_selection(view.id, Selection::single(range.head, range.anchor));
-            align_view(doc, view, Align::Center);
-        }
+    Picker::new(symbols, current_path, move |cx, item, action| {
+        jump_to_location(
+            cx.editor,
+            &item.symbol.location,
+            item.offset_encoding,
+            action,
+        );
     })
     .with_preview(move |_editor, item| Some(location_to_file_location(&item.symbol.location)))
     .truncate_start(false)
@@ -286,7 +263,7 @@ enum DiagnosticsFormat {
 fn diag_picker(
     cx: &Context,
     diagnostics: BTreeMap<lsp::Url, Vec<(lsp::Diagnostic, usize)>>,
-    current_path: Option<lsp::Url>,
+    _current_path: Option<lsp::Url>,
     format: DiagnosticsFormat,
 ) -> Picker<PickerDiagnostic> {
     // TODO: drop current_path comparison and instead use workspace: bool flag?
@@ -324,22 +301,12 @@ fn diag_picker(
                   offset_encoding,
               },
               action| {
-            if current_path.as_ref() == Some(url) {
-                let (view, doc) = current!(cx.editor);
-                push_jump(view, doc);
-            } else {
-                let path = url.to_file_path().unwrap();
-                cx.editor.open(&path, action).expect("editor.open failed");
-            }
-
-            let (view, doc) = current!(cx.editor);
-
-            if let Some(range) = lsp_range_to_range(doc.text(), diag.range, *offset_encoding) {
-                // we flip the range so that the cursor sits on the start of the symbol
-                // (for example start of the function).
-                doc.set_selection(view.id, Selection::single(range.head, range.anchor));
-                align_view(doc, view, Align::Center);
-            }
+            jump_to_location(
+                cx.editor,
+                &lsp::Location::new(url.clone(), diag.range),
+                *offset_encoding,
+                action,
+            )
         },
     )
     .with_preview(move |_editor, PickerDiagnostic { url, diag, .. }| {
-- 
cgit v1.2.3-70-g09d2