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

Trim quotes and braces from paths in goto_file_impl #4370

Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -1032,15 +1033,24 @@ 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 {
the-mikedavis marked this conversation as resolved.
Show resolved Hide resolved
let current_word = movement::move_next_long_word_start(
// Checks whether the current selection is empty
the-mikedavis marked this conversation as resolved.
Show resolved Hide resolved
if selections.len() == 1 && primary.len() == 1 {
let count = cx.count();
// In this case it selects the long word under the cursor
the-mikedavis marked this conversation as resolved.
Show resolved Hide resolved
let current_word = textobject::textobject_word(
text.slice(..),
movement::move_prev_long_word_start(text.slice(..), primary, 1),
1,
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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use current_word.fragment(text) and then .trim_matches on that - it will have a chance of not copying the range's contents until we get to the .to_string() at the bottom (calling .to_string() will always copy the contents).

(For this to work, we'll need to add a line up on L1038 saying let text = text.slice(..) and then we can re-use the ropeslice in the call to textobject_word.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much!

.trim_matches(surrounding_chars)
.to_string(),
);
}
Expand Down
64 changes: 64 additions & 0 deletions helix-term/tests/test/commands.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use std::ops::RangeInclusive;

use helix_core::diagnostic::Severity;
use helix_term::application::Application;
use std::ffi::OsStr;

use super::*;

Expand Down Expand Up @@ -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()
the-mikedavis marked this conversation as resolved.
Show resolved Hide resolved
}

// Single selection
test_key_sequence(
&mut AppBuilder::new().with_file(file.path(), None).build()?,
Some("ione.js<esc>%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.js<ret>two.js<esc>%<A-s>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'<esc>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'<esc>bgf"),
Some(&|app| {
assert_eq!(1, match_paths(app, ["one.js"].to_vec()));
}),
false,
)
.await?;

Ok(())
}