From 5127874f82003c1f79526ceeafe3bed86489f2e7 Mon Sep 17 00:00:00 2001 From: Dario Oddenino Date: Wed, 19 Oct 2022 17:00:58 +0200 Subject: [PATCH 01/10] Fix #4122 The functions trims surrounding `'`, `"`, `(` and `)` before opening the path. The `current_word` selection was inverted so that now it works from both the first and last characters of the word. --- helix-term/src/commands.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 468e9814d2ec..54c48a1e9b5f 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1033,9 +1033,9 @@ fn goto_file_impl(cx: &mut Context, action: Action) { .collect(); let primary = selections.primary(); if selections.len() == 1 && primary.to() - primary.from() == 1 { - let current_word = movement::move_next_long_word_start( + let current_word = movement::move_prev_long_word_start( text.slice(..), - movement::move_prev_long_word_start(text.slice(..), primary, 1), + movement::move_next_long_word_start(text.slice(..), primary, 1), 1, ); paths.clear(); @@ -1045,7 +1045,8 @@ fn goto_file_impl(cx: &mut Context, action: Action) { ); } for sel in paths { - let p = sel.trim(); + let surrounding_chars: &[_] = &['\'', '"', '(', ')']; + let p = sel.trim().trim_matches(surrounding_chars); if !p.is_empty() { if let Err(e) = cx.editor.open(&PathBuf::from(p), action) { cx.editor.set_error(format!("Open file failed: {:?}", e)); From e4955b201ba83fe3b61063bd1cb05240bac8073d Mon Sep 17 00:00:00 2001 From: Dario Oddenino Date: Thu, 20 Oct 2022 09:59:50 +0200 Subject: [PATCH 02/10] fix word selection --- helix-term/src/commands.rs | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 54c48a1e9b5f..a6a96a8c0269 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1033,16 +1033,20 @@ fn goto_file_impl(cx: &mut Context, action: Action) { .collect(); let primary = selections.primary(); if selections.len() == 1 && primary.to() - primary.from() == 1 { - let current_word = movement::move_prev_long_word_start( - text.slice(..), - movement::move_next_long_word_start(text.slice(..), primary, 1), - 1, - ); - paths.clear(); - paths.push( - text.slice(current_word.from()..current_word.to()) - .to_string(), - ); + let count = cx.count(); + let new_selection = selections.clone().transform(|range| { + textobject::textobject_word( + text.slice(..), + range, + textobject::TextObject::Inside, + count, + true, + ) + }); + paths = new_selection + .iter() + .map(|r| text.slice(r.from()..r.to()).to_string()) + .collect(); } for sel in paths { let surrounding_chars: &[_] = &['\'', '"', '(', ')']; From 0e3137dc46c9b7952afd1893ccef71e85d2f4f19 Mon Sep 17 00:00:00 2001 From: Dario Oddenino Date: Fri, 21 Oct 2022 09:51:27 +0200 Subject: [PATCH 03/10] Changed the iter() to primary() --- helix-term/src/commands.rs | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index a6a96a8c0269..4081ba42609b 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1032,21 +1032,25 @@ fn goto_file_impl(cx: &mut Context, action: Action) { .map(|r| text.slice(r.from()..r.to()).to_string()) .collect(); let primary = selections.primary(); - if selections.len() == 1 && primary.to() - primary.from() == 1 { + if primary.len() == 1 { let count = cx.count(); - let new_selection = selections.clone().transform(|range| { - textobject::textobject_word( - text.slice(..), - range, - textobject::TextObject::Inside, - count, - true, - ) - }); - paths = new_selection - .iter() - .map(|r| text.slice(r.from()..r.to()).to_string()) - .collect(); + let current_word = selections + .clone() + .transform(|range| { + textobject::textobject_word( + text.slice(..), + range, + textobject::TextObject::Inside, + count, + true, + ) + }) + .primary(); + paths.clear(); + paths.push( + text.slice(current_word.from()..current_word.to()) + .to_string(), + ); } for sel in paths { let surrounding_chars: &[_] = &['\'', '"', '(', ')']; From 68769627393b70332eb5580621dd6037d61ded5f Mon Sep 17 00:00:00 2001 From: Dario Oddenino Date: Mon, 24 Oct 2022 10:20:12 +0200 Subject: [PATCH 04/10] Improvements and comments --- helix-term/src/commands.rs | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 4081ba42609b..d560bfa0e265 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1023,6 +1023,7 @@ fn goto_file_vsplit(cx: &mut Context) { goto_file_impl(cx, Action::VerticalSplit); } +/// Goto files in selection. fn goto_file_impl(cx: &mut Context, action: Action) { let (view, doc) = current_ref!(cx.editor); let text = doc.text(); @@ -1032,30 +1033,30 @@ fn goto_file_impl(cx: &mut Context, action: Action) { .map(|r| text.slice(r.from()..r.to()).to_string()) .collect(); let primary = selections.primary(); - if primary.len() == 1 { + // Checks whether the current selection is empty + if selections.len() == 1 && primary.len() == 1 { let count = cx.count(); - let current_word = selections - .clone() - .transform(|range| { - textobject::textobject_word( - text.slice(..), - range, - textobject::TextObject::Inside, - count, - true, - ) - }) - .primary(); + // In this case it selects the long word under the cursor + let current_word = textobject::textobject_word( + text.slice(..), + primary, + textobject::TextObject::Inside, + count, + true, + ); + // Trims some surrounding chars so that the actual file is opened. + let surrounding_chars: &[_] = &['\'', '"', '(', ')']; paths.clear(); paths.push( text.slice(current_word.from()..current_word.to()) + .to_string() + .trim_matches(surrounding_chars) .to_string(), ); } for sel in paths { - let surrounding_chars: &[_] = &['\'', '"', '(', ')']; - let p = sel.trim().trim_matches(surrounding_chars); - if !p.is_empty() { + if !sel.is_empty() { + let p = sel.trim(); if let Err(e) = cx.editor.open(&PathBuf::from(p), action) { cx.editor.set_error(format!("Open file failed: {:?}", e)); } From f14159c8b4bdb195d2959fb4a4385ba95a7511f5 Mon Sep 17 00:00:00 2001 From: Dario Oddenino Date: Mon, 24 Oct 2022 10:26:59 +0200 Subject: [PATCH 05/10] hotfix --- helix-term/src/commands.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index d560bfa0e265..71a61f5a6078 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1055,8 +1055,8 @@ fn goto_file_impl(cx: &mut Context, action: Action) { ); } for sel in paths { - if !sel.is_empty() { - let p = sel.trim(); + let p = sel.trim(); + if !p.is_empty() { if let Err(e) = cx.editor.open(&PathBuf::from(p), action) { cx.editor.set_error(format!("Open file failed: {:?}", e)); } From 432b1ae1f77f8145d8ac5422646c96c67e275611 Mon Sep 17 00:00:00 2001 From: Dario Oddenino Date: Mon, 24 Oct 2022 13:27:29 +0200 Subject: [PATCH 06/10] tests! test fix --- helix-term/tests/test/commands.rs | 64 +++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index e24ee3e08af6..44336d08100d 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -1,6 +1,8 @@ use std::ops::RangeInclusive; use helix_core::diagnostic::Severity; +use helix_term::application::Application; +use std::ffi::OsStr; use super::*; @@ -133,3 +135,65 @@ async fn test_selection_duplication() -> anyhow::Result<()> { .await?; Ok(()) } + +#[tokio::test(flavor = "multi_thread")] +async fn test_goto_file_impl() -> anyhow::Result<()> { + let file = tempfile::NamedTempFile::new()?; + + fn match_paths(app: &Application, matches: Vec<&str>) -> usize { + app.editor.documents() + .map(|d| d.path()) + .flatten() + .map(|p| p.file_name()) + .flatten() + .filter(|n| matches.iter().any(|m| &OsStr::new(m) == n)) + .count() + } + + // Single selection + test_key_sequence( + &mut AppBuilder::new().with_file(file.path(), None).build()?, + Some("ione.js%gf"), + Some(&|app| { + assert_eq!(1, match_paths(app, ["one.js"].to_vec())); + }), + false, + ) + .await?; + + // Multiple selection + test_key_sequence( + &mut AppBuilder::new().with_file(file.path(), None).build()?, + Some("ione.jstwo.js%gf"), + Some(&|app| { + assert_eq!(2, match_paths(app, ["one.js", "two.js"].to_vec())); + }), + false, + ) + .await?; + + // Cursor on first quote + test_key_sequence( + &mut AppBuilder::new().with_file(file.path(), None).build()?, + Some("iimport 'one.js'B;gf"), + Some(&|app| { + assert_eq!(1, match_paths(app, ["one.js"].to_vec())); + }), + false, + ) + .await?; + + // Cursor on last quote + test_key_sequence( + &mut AppBuilder::new().with_file(file.path(), None).build()?, + Some("iimport 'one.js'bgf"), + Some(&|app| { + assert_eq!(1, match_paths(app, ["one.js"].to_vec())); + }), + false, + ) + .await?; + + + Ok(()) +} \ No newline at end of file From 956b1ea93057b3a2836ca0df9d44da3cd2baee5d Mon Sep 17 00:00:00 2001 From: Dario Oddenino Date: Mon, 24 Oct 2022 13:40:29 +0200 Subject: [PATCH 07/10] formatting --- helix-term/tests/test/commands.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index 44336d08100d..ac88b49723fa 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -139,9 +139,10 @@ async fn test_selection_duplication() -> anyhow::Result<()> { #[tokio::test(flavor = "multi_thread")] async fn test_goto_file_impl() -> anyhow::Result<()> { let file = tempfile::NamedTempFile::new()?; - + fn match_paths(app: &Application, matches: Vec<&str>) -> usize { - app.editor.documents() + app.editor + .documents() .map(|d| d.path()) .flatten() .map(|p| p.file_name()) @@ -149,7 +150,7 @@ async fn test_goto_file_impl() -> anyhow::Result<()> { .filter(|n| matches.iter().any(|m| &OsStr::new(m) == n)) .count() } - + // Single selection test_key_sequence( &mut AppBuilder::new().with_file(file.path(), None).build()?, @@ -160,7 +161,7 @@ async fn test_goto_file_impl() -> anyhow::Result<()> { false, ) .await?; - + // Multiple selection test_key_sequence( &mut AppBuilder::new().with_file(file.path(), None).build()?, @@ -171,7 +172,7 @@ async fn test_goto_file_impl() -> anyhow::Result<()> { false, ) .await?; - + // Cursor on first quote test_key_sequence( &mut AppBuilder::new().with_file(file.path(), None).build()?, @@ -182,7 +183,7 @@ async fn test_goto_file_impl() -> anyhow::Result<()> { false, ) .await?; - + // Cursor on last quote test_key_sequence( &mut AppBuilder::new().with_file(file.path(), None).build()?, @@ -193,7 +194,6 @@ async fn test_goto_file_impl() -> anyhow::Result<()> { false, ) .await?; - - + Ok(()) -} \ No newline at end of file +} From 80cbdb912bf03e84d2195be4bc6f161718665cf6 Mon Sep 17 00:00:00 2001 From: Dario Oddenino Date: Tue, 25 Oct 2022 09:58:01 +0200 Subject: [PATCH 08/10] fixes --- helix-term/src/commands.rs | 4 ++-- helix-term/tests/test/commands.rs | 8 ++------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 71a61f5a6078..88bf5cb91794 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1033,10 +1033,10 @@ fn goto_file_impl(cx: &mut Context, action: Action) { .map(|r| text.slice(r.from()..r.to()).to_string()) .collect(); let primary = selections.primary(); - // Checks whether the current selection is empty + // Checks whether there is only one selection with a width of 1 if selections.len() == 1 && primary.len() == 1 { let count = cx.count(); - // In this case it selects the long word under the cursor + // In this case it selects the WORD under the cursor let current_word = textobject::textobject_word( text.slice(..), primary, diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index ac88b49723fa..41c1299766e5 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -2,7 +2,6 @@ use std::ops::RangeInclusive; use helix_core::diagnostic::Severity; use helix_term::application::Application; -use std::ffi::OsStr; use super::*; @@ -143,11 +142,8 @@ async fn test_goto_file_impl() -> anyhow::Result<()> { fn match_paths(app: &Application, matches: Vec<&str>) -> usize { app.editor .documents() - .map(|d| d.path()) - .flatten() - .map(|p| p.file_name()) - .flatten() - .filter(|n| matches.iter().any(|m| &OsStr::new(m) == n)) + .filter_map(|d| d.path()?.file_name()) + .filter(|n| matches.iter().any(|m| *m == n.to_string_lossy())) .count() } From 27dfebd46e896559f7ed368129b0207117f1b9de Mon Sep 17 00:00:00 2001 From: Dario Oddenino Date: Wed, 26 Oct 2022 09:57:36 +0200 Subject: [PATCH 09/10] Remove extra to_string --- helix-term/src/commands.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 88bf5cb91794..a677feaf00ac 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -1036,9 +1036,10 @@ fn goto_file_impl(cx: &mut Context, action: Action) { // Checks whether there is only one selection with a width of 1 if selections.len() == 1 && primary.len() == 1 { let count = cx.count(); + let text_slice = text.slice(..); // In this case it selects the WORD under the cursor let current_word = textobject::textobject_word( - text.slice(..), + text_slice, primary, textobject::TextObject::Inside, count, @@ -1048,8 +1049,8 @@ fn goto_file_impl(cx: &mut Context, action: Action) { let surrounding_chars: &[_] = &['\'', '"', '(', ')']; paths.clear(); paths.push( - text.slice(current_word.from()..current_word.to()) - .to_string() + current_word + .fragment(text_slice) .trim_matches(surrounding_chars) .to_string(), ); From c8c4dc023b625f8560c9e8dce7b5c2f80c07cba2 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Thu, 27 Oct 2022 20:22:29 -0500 Subject: [PATCH 10/10] Use vec! macro for expected paths in integration test --- helix-term/tests/test/commands.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index 41c1299766e5..aadf104bb427 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -152,7 +152,7 @@ async fn test_goto_file_impl() -> anyhow::Result<()> { &mut AppBuilder::new().with_file(file.path(), None).build()?, Some("ione.js%gf"), Some(&|app| { - assert_eq!(1, match_paths(app, ["one.js"].to_vec())); + assert_eq!(1, match_paths(app, vec!["one.js"])); }), false, ) @@ -163,7 +163,7 @@ async fn test_goto_file_impl() -> anyhow::Result<()> { &mut AppBuilder::new().with_file(file.path(), None).build()?, Some("ione.jstwo.js%gf"), Some(&|app| { - assert_eq!(2, match_paths(app, ["one.js", "two.js"].to_vec())); + assert_eq!(2, match_paths(app, vec!["one.js", "two.js"])); }), false, ) @@ -174,7 +174,7 @@ async fn test_goto_file_impl() -> anyhow::Result<()> { &mut AppBuilder::new().with_file(file.path(), None).build()?, Some("iimport 'one.js'B;gf"), Some(&|app| { - assert_eq!(1, match_paths(app, ["one.js"].to_vec())); + assert_eq!(1, match_paths(app, vec!["one.js"])); }), false, ) @@ -185,7 +185,7 @@ async fn test_goto_file_impl() -> anyhow::Result<()> { &mut AppBuilder::new().with_file(file.path(), None).build()?, Some("iimport 'one.js'bgf"), Some(&|app| { - assert_eq!(1, match_paths(app, ["one.js"].to_vec())); + assert_eq!(1, match_paths(app, vec!["one.js"])); }), false, )