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.

Sign up for GitHub

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

Jump to bottom

Let gf go to file with line number #9519

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Let gf go to file with line number #9519

wants to merge 2 commits into from

Conversation

scdailey
Copy link

@scdailey scdailey commented Feb 4, 2024 •
edited by the-mikedavis
Loading

Fixes #9440

@scdailey scdailey mentioned this pull request Feb 4, 2024
Open
@the-mikedavis the-mikedavis added S-waiting-on-review Status: Awaiting review from a maintainer. A-command Area: Commands labels Feb 4, 2024
@kirawi kirawi added the C-enhancement Category: Improvements label Feb 7, 2024
the-mikedavis
the-mikedavis requested changes Feb 10, 2024
View reviewed changes
helix-term/src/commands.rs
@@ -1162,10 +1162,6 @@ fn goto_file_impl(cx: &mut Context, action: Action) {
let (view, doc) = current_ref!(cx.editor);
let text = doc.text();
let selections = doc.selection(view.id);
let rel_path = doc
Copy link
Member

Choose a reason for hiding this comment

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

I believe this changes the behavior of gf a little: the path should be interpreted as relative to the current document. Also see #8126 which aims to change that behavior

Copy link
Author

Choose a reason for hiding this comment

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

Okay I can update it. Just to clarify, are we going with the original wording of that issue (not sure if it even applies still) or the clarification here:

#8126 (comment)

Just resolve relative to the current file path no matter what?

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer the behavior from Pascal's comment. I think this makes sense:

  1. Check if the file exists relative to the current doc
  2. Check if the file exists relative to the CWD
  3. Otherwise open a new file relative to the current doc

@the-mikedavis the-mikedavis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from a maintainer. labels Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@the-mikedavis the-mikedavis the-mikedavis requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
A-command Area: Commands C-enhancement Category: Improvements S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Let gf go to file with line number
3 participants
@scdailey @the-mikedavis @kirawi