git.iko.soy/helix
on master help sign in
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>

commitaa4d84a0b30fbe93882508c6d9dc8ca4c95341d0 authorwoojiq <122799969+woojiq@users.noreply.github.com> dateTue 8 Aug 2023, 16:17 (3 years ago) parentc1c71bb
5 files changed+121−63
compare with master
unifiedsplit
helix-term/src/commands.rs+11−8 helix-term/src/commands/lsp.rs+22−55 helix-term/tests/integration.rs+1 helix-term/tests/test/picker.rs+80 helix-view/src/editor.rs+7
helix-term/src/commands.rs+11−8
@@ -2199,8 +2199,8 @@ fn global_search(cx: &mut Context) {
21992199 all_matches,
22002200 current_path,
22012201 move |cx, FileResult { path, line_num }, action| {
2202− match cx.editor.open(path, action) {
2203− Ok(_) => {}
2202+ let doc = match cx.editor.open(path, action) {
2203+ Ok(id) => doc_mut!(cx.editor, &id),
22042204 Err(e) => {
22052205 cx.editor.set_error(format!(
22062206 "Failed to open file '{}': {}",
@@ -2209,10 +2209,9 @@ fn global_search(cx: &mut Context) {
22092209 ));
22102210 return;
22112211 }
2212− }
2213−
2212+ };
22142213 let line_num = *line_num;
2215− let (view, doc) = current!(cx.editor);
2214+ let view = view_mut!(cx.editor);
22162215 let text = doc.text();
22172216 if line_num >= text.len_lines() {
22182217 cx.editor.set_error("The line you jumped to does not exist anymore because the file has changed.");
@@ -2222,7 +2221,9 @@ fn global_search(cx: &mut Context) {
22222221 let end = text.line_to_char((line_num + 1).min(text.len_lines()));
22232222
22242223 doc.set_selection(view.id, Selection::single(start, end));
2225− align_view(doc, view, Align::Center);
2224+ if action.align_view(view, doc.id()){
2225+ align_view(doc, view, Align::Center);
2226+ }
22262227 }).with_preview(|_editor, FileResult { path, line_num }| {
22272228 Some((path.clone().into(), Some((*line_num, *line_num))))
22282229 });
@@ -2742,9 +2743,11 @@ fn jumplist_picker(cx: &mut Context) {
27422743 |cx, meta, action| {
27432744 cx.editor.switch(meta.id, action);
27442745 let config = cx.editor.config();
2745− let (view, doc) = current!(cx.editor);
2746+ let (view, doc) = (view_mut!(cx.editor), doc_mut!(cx.editor, &meta.id));
27462747 doc.set_selection(view.id, meta.selection.clone());
2747− view.ensure_cursor_in_view_center(doc, config.scrolloff);
2748+ if action.align_view(view, doc.id()) {
2749+ view.ensure_cursor_in_view_center(doc, config.scrolloff);
2750+ }
27482751 },
27492752 )
27502753 .with_preview(|editor, meta| {
helix-term/src/commands/lsp.rs+22−55
@@ -195,7 +195,6 @@ fn location_to_file_location(location: &lsp::Location) -> FileLocation {
195195 (path.into(), line)
196196}
197197
198−// TODO: share with symbol picker(symbol.location)
199198fn jump_to_location(
200199 editor: &mut Editor,
201200 location: &lsp::Location,
@@ -213,15 +212,16 @@ fn jump_to_location(
213212 return;
214213 }
215214 };
216− match editor.open(&path, action) {
217− Ok(_) => (),
215+
216+ let doc = match editor.open(&path, action) {
217+ Ok(id) => doc_mut!(editor, &id),
218218 Err(err) => {
219219 let err = format!("failed to open path: {:?}: {:?}", location.uri, err);
220220 editor.set_error(err);
221221 return;
222222 }
223− }
224− let (view, doc) = current!(editor);
223+ };
224+ let view = view_mut!(editor);
225225 // TODO: convert inside server
226226 let new_range =
227227 if let Some(new_range) = lsp_range_to_range(doc.text(), location.range, offset_encoding) {
@@ -233,45 +233,22 @@ fn jump_to_location(
233233 // we flip the range so that the cursor sits on the start of the symbol
234234 // (for example start of the function).
235235 doc.set_selection(view.id, Selection::single(new_range.head, new_range.anchor));
236− align_view(doc, view, Align::Center);
236+ if action.align_view(view, doc.id()) {
237+ align_view(doc, view, Align::Center);
238+ }
237239}
238240
239241type SymbolPicker = Picker<SymbolInformationItem>;
240242
241243fn sym_picker(symbols: Vec<SymbolInformationItem>, current_path: Option<lsp::Url>) -> SymbolPicker {
242244 // TODO: drop current_path comparison and instead use workspace: bool flag?
243− Picker::new(symbols, current_path.clone(), move |cx, item, action| {
244− let (view, doc) = current!(cx.editor);
245− push_jump(view, doc);
246−
247− if current_path.as_ref() != Some(&item.symbol.location.uri) {
248− let uri = &item.symbol.location.uri;
249− let path = match uri.to_file_path() {
250− Ok(path) => path,
251− Err(_) => {
252− let err = format!("unable to convert URI to filepath: {}", uri);
253− cx.editor.set_error(err);
254− return;
255− }
256− };
257− if let Err(err) = cx.editor.open(&path, action) {
258− let err = format!("failed to open document: {}: {}", uri, err);
259− log::error!("{}", err);
260− cx.editor.set_error(err);
261− return;
262− }
263− }
264−
265− let (view, doc) = current!(cx.editor);
266−
267− if let Some(range) =
268− lsp_range_to_range(doc.text(), item.symbol.location.range, item.offset_encoding)
269− {
270− // we flip the range so that the cursor sits on the start of the symbol
271− // (for example start of the function).
272− doc.set_selection(view.id, Selection::single(range.head, range.anchor));
273− align_view(doc, view, Align::Center);
274− }
245+ Picker::new(symbols, current_path, move |cx, item, action| {
246+ jump_to_location(
247+ cx.editor,
248+ &item.symbol.location,
249+ item.offset_encoding,
250+ action,
251+ );
275252 })
276253 .with_preview(move |_editor, item| Some(location_to_file_location(&item.symbol.location)))
277254 .truncate_start(false)
@@ -286,7 +263,7 @@ enum DiagnosticsFormat {
286263fn diag_picker(
287264 cx: &Context,
288265 diagnostics: BTreeMap<lsp::Url, Vec<(lsp::Diagnostic, usize)>>,
289− current_path: Option<lsp::Url>,
266+ _current_path: Option<lsp::Url>,
290267 format: DiagnosticsFormat,
291268) -> Picker<PickerDiagnostic> {
292269 // TODO: drop current_path comparison and instead use workspace: bool flag?
@@ -324,22 +301,12 @@ fn diag_picker(
324301 offset_encoding,
325302 },
326303 action| {
327− if current_path.as_ref() == Some(url) {
328− let (view, doc) = current!(cx.editor);
329− push_jump(view, doc);
330− } else {
331− let path = url.to_file_path().unwrap();
332− cx.editor.open(&path, action).expect("editor.open failed");
333− }
334−
335− let (view, doc) = current!(cx.editor);
336−
337− if let Some(range) = lsp_range_to_range(doc.text(), diag.range, *offset_encoding) {
338− // we flip the range so that the cursor sits on the start of the symbol
339− // (for example start of the function).
340− doc.set_selection(view.id, Selection::single(range.head, range.anchor));
341− align_view(doc, view, Align::Center);
342− }
304+ jump_to_location(
305+ cx.editor,
306+ &lsp::Location::new(url.clone(), diag.range),
307+ *offset_encoding,
308+ action,
309+ )
343310 },
344311 )
345312 .with_preview(move |_editor, PickerDiagnostic { url, diag, .. }| {
helix-term/tests/integration.rs+1
@@ -19,6 +19,7 @@ mod test {
1919 mod auto_pairs;
2020 mod commands;
2121 mod movement;
22+ mod picker;
2223 mod prompt;
2324 mod splits;
2425}
helix-term/tests/test/picker.rsnew file+80
@@ -0,0 +1,80 @@
1+use std::fs;
2+
3+use helix_core::{path::get_canonicalized_path, Range};
4+use helix_loader::{current_working_dir, set_current_working_dir};
5+use helix_view::{current_ref, editor::Action};
6+use tempfile::{Builder, TempDir};
7+
8+use super::*;
9+
10+#[tokio::test(flavor = "multi_thread")]
11+async fn test_picker_alt_ret() -> anyhow::Result<()> {
12+ // Create two files, open the first and run a global search for a word
13+ // from the second file. Press <alt-ret> to have helix open the second file in the
14+ // new buffer, but not change focus. Then check whether the word is highlighted
15+ // correctly and the view of the first file has not changed.
16+ let tmp_dir = TempDir::new()?;
17+ set_current_working_dir(tmp_dir.path().into())?;
18+
19+ let mut app = AppBuilder::new().build()?;
20+
21+ log::debug!(
22+ "set current working directory to {:?}",
23+ current_working_dir()
24+ );
25+
26+ // Add prefix so helix doesn't hide these files in a picker
27+ let files = [
28+ Builder::new().prefix("1").tempfile_in(&tmp_dir)?,
29+ Builder::new().prefix("2").tempfile_in(&tmp_dir)?,
30+ ];
31+ let paths = files
32+ .iter()
33+ .map(|f| get_canonicalized_path(f.path()).unwrap())
34+ .collect::<Vec<_>>();
35+
36+ fs::write(&paths[0], "1\n2\n3\n4")?;
37+ fs::write(&paths[1], "first\nsecond")?;
38+
39+ log::debug!(
40+ "created and wrote two temporary files: {:?} & {:?}",
41+ paths[0],
42+ paths[1]
43+ );
44+
45+ // Manually open to save the offset, otherwise we won't be able to change the state in the Fn trait
46+ app.editor.open(files[0].path(), Action::Replace)?;
47+ let view_offset = current_ref!(app.editor).0.offset;
48+
49+ test_key_sequences(
50+ &mut app,
51+ vec![
52+ (Some("<space>/"), None),
53+ (Some("second<ret>"), None),
54+ (
55+ Some("<A-ret><esc>"),
56+ Some(&|app| {
57+ let (view, doc) = current_ref!(app.editor);
58+ assert_eq!(doc.path().unwrap(), &paths[0]);
59+ let select_ranges = doc.selection(view.id).ranges();
60+ assert_eq!(select_ranges[0], Range::new(0, 1));
61+ assert_eq!(view.offset, view_offset);
62+ }),
63+ ),
64+ (
65+ Some(":buffer<minus>next<ret>"),
66+ Some(&|app| {
67+ let (view, doc) = current_ref!(app.editor);
68+ assert_eq!(doc.path().unwrap(), &paths[1]);
69+ let select_ranges = doc.selection(view.id).ranges();
70+ assert_eq!(select_ranges.len(), 1);
71+ assert_eq!(select_ranges[0], Range::new(6, 12));
72+ }),
73+ ),
74+ ],
75+ false,
76+ )
77+ .await?;
78+
79+ Ok(())
80+}
helix-view/src/editor.rs+7
@@ -995,6 +995,13 @@ pub enum Action {
995995 VerticalSplit,
996996}
997997
998+impl Action {
999+ /// Whether to align the view to the cursor after executing this action
1000+ pub fn align_view(&self, view: &View, new_doc: DocumentId) -> bool {
1001+ !matches!((self, view.doc == new_doc), (Action::Load, false))
1002+ }
1003+}
1004+
9981005/// Error thrown on failed document closed
9991006pub enum CloseError {
10001007 /// Document doesn't exist