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

Improve goto_file_impl #9065

Merged
merged 23 commits into from
Apr 6, 2024

Conversation

TornaxO7
Copy link
Contributor

continues #9038

Closes #8822 #8824

@TornaxO7 TornaxO7 marked this pull request as draft December 12, 2023 21:32
@TornaxO7 TornaxO7 force-pushed the feat/extend-goto-file branch from 5b05bbb to eab5cbd Compare December 12, 2023 21:33
@TornaxO7
Copy link
Contributor Author

@ModProg

may I ask if you can try it out?

@TornaxO7 TornaxO7 changed the title adding shellexpand and extend surround_chars improve goto_file_impl Dec 12, 2023
@TornaxO7 TornaxO7 marked this pull request as ready for review December 13, 2023 22:02
@TornaxO7
Copy link
Contributor Author

TornaxO7 commented Dec 13, 2023

File path detection strategy

I decided to read the chars next to the cursor one by one because:

  1. using treesitter requires a parser which doesn't exist for text or log files
    although paths could occur there
  2. paths are short enough to be read char by char in my opinion without waiting too
    long

Other information

So I tested it on $HOME/test.txt and ~/test.txt. Both opened up ~/test.txt as expected
but I don't know how to write tests for them.

@ModProg
Copy link

ModProg commented Dec 13, 2023

I'd consider adding , to the characters terminating a path (like ;).

@TornaxO7
Copy link
Contributor Author

I'd consider adding , to the characters terminating a path (like ;).

added

Comment on lines 1183 to 1190
&[
'@', '/', '\\', '.', '-', '_', '+', '#', '$', '%', '{', '}', '[', ']', ':',
'!', '~', '=',
]
} else {
&['@', '/', '.', '-', '_', '+', '#', '$', '%', '~', '=']
};
valid_chars.contains(c) || c.is_alphabetic()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked that up from neovim. See :h isf in neovim for the list of chars.

@kirawi kirawi added C-enhancement Category: Improvements A-command Area: Commands labels Dec 22, 2023
helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
@pascalkuthe pascalkuthe added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 8, 2024
@TornaxO7 TornaxO7 requested a review from pascalkuthe January 11, 2024 17:36
@the-mikedavis the-mikedavis added S-waiting-on-review Status: Awaiting review from a maintainer. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 10, 2024
helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Outdated Show resolved Hide resolved
@the-mikedavis the-mikedavis removed the S-waiting-on-review Status: Awaiting review from a maintainer. label Feb 10, 2024
@TornaxO7 TornaxO7 force-pushed the feat/extend-goto-file branch from 43c7816 to c5c4c3e Compare February 24, 2024 18:48
@TornaxO7
Copy link
Contributor Author

I can't execute the integration tests. Executing cargo integration-test gives me:

error[E0412]: cannot find type `Id` in module `tokio::runtime`
  --> helix-event/src/runtime.rs:45:64
   |
45 |         parking_lot::RwLock<hashbrown::HashMap<tokio::runtime::Id, &'static T, ahash::RandomState>>,
   |                                                                ^^ not found in `tokio::runtime`

   Compiling gix-tempfile v13.0.0
error[E0599]: no method named `id` found for struct `Handle` in the current scope
  --> helix-event/src/runtime.rs:67:52
   |
67 |         let id = tokio::runtime::Handle::current().id();
   |                                                    ^^ method not found in `Handle`

Some errors have detailed explanations: E0412, E0599.
For more information about an error, try `rustc --explain E0412`.
error: could not compile `helix-event` (lib) due to 2 previous errors
warning: build failed, waiting for other jobs to finish...

@the-mikedavis the-mikedavis added S-waiting-on-review Status: Awaiting review from a maintainer. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 24, 2024
@TornaxO7 TornaxO7 requested a review from pascalkuthe February 27, 2024 21:32
@TornaxO7 TornaxO7 requested a review from the-mikedavis April 6, 2024 19:35
Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

Thanks!

.count();

let path: Cow<str> = text
.slice((start_pos - prefix_len)..(start_pos + postfix_len))
Copy link
Member

Choose a reason for hiding this comment

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

For readers' context: rope slicing is by character index rather than byte index (like &str) which is why this uses chars_at(..).count() rather than a len of a substring

@the-mikedavis the-mikedavis changed the title improve goto_file_impl Improve goto_file_impl Apr 6, 2024
@the-mikedavis the-mikedavis merged commit e69292e into helix-editor:master Apr 6, 2024
6 checks passed
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 20, 2024
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
Vulpesx pushed a commit to Vulpesx/helix that referenced this pull request Jun 7, 2024
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-command Area: Commands C-enhancement Category: Improvements S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

goto_file path with ~
5 participants