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

Feat/extend goto file #9038

Closed
wants to merge 0 commits into from

Conversation

TornaxO7
Copy link
Contributor

@TornaxO7 TornaxO7 commented Dec 9, 2023

Closes #8822 #8824

I'm unsure how to write a test with:

  • environment-variables
  • ~

Also executing gf with:

let a = "some/path".to_string();

will interpret it as some/path".to_string. The only solution which I can think of, is manually collecting all "valid" characters on the right and on the left of the cursor, should I do it like that?

I also added a new config-option: goto_file_ignore_unknown_env_var.
The reason is, that I'm unsure how to let helix behave, if the env-var like $FOO isn't set, but helix should go to the file $FOO/yay.txt. Should it just throw an error and don't enter the file or should it still enter the file? That's what the option provides: Saying that it should treat the path as a normal path, if an env-variable isn't set.
But tbh. I'm unsure if this config-option is needed, so feel free to say, that I should remove it.

@TornaxO7 TornaxO7 marked this pull request as draft December 9, 2023 16:44
// path ends with ","
test_key_sequence(
&mut AppBuilder::new().with_file(file.path(), None).build()?,
Some("i/tmp/test.txt,<esc>bgf"),
Copy link

Choose a reason for hiding this comment

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

This test seems a bit strange, the cursor is on the , when triggering gf, meaning a character that is excluded from the output path.

@ModProg
Copy link

ModProg commented Dec 9, 2023

will interpret it as some/path".to_string. The only solution which I can think of, is manually collecting all "valid" characters on the right and on the left of the cursor, should I do it like that?

On non selection gfs, there are two approaches, one would be to have a constant that defines a set of characters that are allowed/disallowed in paths, and you just go left and right until you find one that matches. This might result in poor performance in some cases, not allowing paths to contain ",;' … at all, so it might make sense to have a more advanced matcher that allows them under certain conditions, e.g., allows ,; if they are not followed by whitespace, e.g., hello,world.txt would be one path, but hello, world.txt would be two, hello and world.txt.

This would require to split the non-path characters in “terminates if neighboring whitespace”, e.g., ,; “terminates only on rhs and if followed by whitespace”, e.g., . and “terminates always”, e.g., " '.

One thing one might consider, is using treesitter if available, this would allow actual string support, and could even support spaces. This is probably more of a future extension of this, as it would only work when treesitter is supported for the file at hand.

@TornaxO7
Copy link
Contributor Author

Hm... good point, I'll try to make use of tree-sitter here

@kirawi
Copy link
Member

kirawi commented Dec 11, 2023

You can see how tree-sitter is used for matching brackets as a stepping stone:

fn find_pair(
syntax: &Syntax,
doc: RopeSlice,
pos_: usize,
traverse_parents: bool,
) -> Option<usize> {
let tree = syntax.tree();
let pos = doc.char_to_byte(pos_);
let mut node = tree.root_node().descendant_for_byte_range(pos, pos)?;
loop {
if node.is_named() {
let (start_byte, end_byte) = surrounding_bytes(doc, &node)?;
let (start_char, end_char) = (doc.byte_to_char(start_byte), doc.byte_to_char(end_byte));
if is_valid_pair(doc, start_char, end_char) {
if end_byte == pos {
return Some(start_char);
}
// We return the end char if the cursor is either on the start char
// or at some arbitrary position between start and end char.
if traverse_parents || start_byte == pos {
return Some(end_char);
}
}
}
// this node itselt wasn't a pair but maybe its siblings are
// check if we are *on* the pair (special cased so we don't look
// at the current node twice and to jump to the start on that case)
if let Some(open) = as_close_pair(doc, &node) {
if let Some(pair_start) = find_pair_end(doc, node.prev_sibling(), open, Backward) {
return Some(pair_start);
}
}
if !traverse_parents {
// check if we are *on* the opening pair (special cased here as
// an opptimization since we only care about bracket on the cursor
// here)
if let Some(close) = as_open_pair(doc, &node) {
if let Some(pair_end) = find_pair_end(doc, node.next_sibling(), close, Forward) {
return Some(pair_end);
}
}
if node.is_named() {
break;
}
}
for close in
iter::successors(node.next_sibling(), |node| node.next_sibling()).take(MATCH_LIMIT)
{
let Some(open) = as_close_pair(doc, &close) else {
continue;
};
if find_pair_end(doc, Some(node), open, Backward).is_some() {
return doc.try_byte_to_char(close.start_byte()).ok();
}
}
let Some(parent) = node.parent() else {
break;
};
node = parent;
}
let node = tree.root_node().named_descendant_for_byte_range(pos, pos)?;
if node.child_count() != 0 {
return None;
}
let node_start = doc.byte_to_char(node.start_byte());
find_matching_bracket_plaintext(doc.byte_slice(node.byte_range()), pos_ - node_start)
.map(|pos| pos + node_start)
}

@TornaxO7
Copy link
Contributor Author

TornaxO7 commented Dec 12, 2023

eh, I don't remember that I closed this O.O and why is github telling me that this got merged?
image

@TornaxO7 TornaxO7 mentioned this pull request Dec 12, 2023
@the-mikedavis
Copy link
Member

Github will automatically close PRs if it doesn't see any difference between the PR's branch and the target branch. It looks like you force-pushed the branch to 5109286 which is a commit from master

@TornaxO7
Copy link
Contributor Author

oooh ok, thank you for the information!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

goto_file path with ~
4 participants