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

[BUG] Go to file is capturing surrounding single quotes #4122

Closed
lukepighetti opened this issue Oct 6, 2022 · 3 comments · Fixed by #4370
Closed

[BUG] Go to file is capturing surrounding single quotes #4122

lukepighetti opened this issue Oct 6, 2022 · 3 comments · Fixed by #4370
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug

Comments

@lukepighetti
Copy link

lukepighetti commented Oct 6, 2022

files

index.js
example.js

consider a file example.js that contains the line
import './index.js' or
import 'index.js'

You cannot navigate to index.js by using gf on either line

What's interesting is you can manually select index.js and hit gf and it works.

It looks like gf is capturing the surrounding single quotes. Neovim does not exhibit that behavior.

So in Helix you have to mi'gf and in Neovim you do gf

@lukepighetti lukepighetti changed the title Dot prefixed files cannot be navigated do with Go to file [BUG] Go to file is capturing surrounding single quotes Oct 6, 2022
@sudormrfbin sudormrfbin added the C-bug Category: This is a bug label Oct 6, 2022
@kirawi kirawi added the A-helix-term Area: Helix term improvements label Oct 9, 2022
@dariooddenino
Copy link
Contributor

Is removing quotes/double quotes from the selection an acceptable approach here?

I wonder if it makes sense to have something "smarter" at work here. In this example one could have import 'index' without the file extension, and with the current approach this would not open the correct file.

@lukepighetti
Copy link
Author

In this particular case I think it's okay to Do It Like Neovim™. It's just too common to drop a cursor on the line import './index.js' hit gf and expect it to open ./index.js

@sudormrfbin
Copy link
Member

When there's only a single cursor with a selection width of 1, the current LONG word is considered as the filename, so it's similar to doing miWgf:

let mut paths: Vec<_> = selections
.iter()
.map(|r| text.slice(r.from()..r.to()).to_string())
.collect();
let primary = selections.primary();
if selections.len() == 1 && primary.to() - primary.from() == 1 {
let current_word = movement::move_next_long_word_start(
text.slice(..),
movement::move_prev_long_word_start(text.slice(..), primary, 1),
1,
);
paths.clear();
paths.push(
text.slice(current_word.from()..current_word.to())
.to_string(),
);
}

We could trim surround character pairs like () and ' before trying to open the file like @dariooddenino suggested.

I wonder if it makes sense to have something "smarter" at work here. In this example one could have import 'index' without the file extension, and with the current approach this would not open the correct file.

Cases like these are better handled by the LSP that has the additional context at hand to resolve the path reliably.

dariooddenino added a commit to dariooddenino/helix that referenced this issue Oct 19, 2022
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.
dariooddenino added a commit to dariooddenino/helix that referenced this issue Oct 24, 2022
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants