Add support for path completion #2608

Philipp-M · 2022-05-29T22:05:45Z

This PR adds support for path completion, currently only unix paths supported.

It supports the following:

Autocompletion is triggered with /.
Documentation preview (file permissions, canonicalized path).
Home-path resolution (~/path)
Link resolution (makes sense for preview, since the LSP specification (CompletionItemKind) only supports files and folders but not symlinks)
Async (via spawn_blocking instead of tokios file accessor functions, as they IMHO make the code less readable and are quite a bit slower than just spawning a "thread")
Configurable with editor.path-completion (default true), per-language overrideable path-completion support

A baby-step towards #1015

kirawi · 2022-05-30T02:18:51Z

I think it would be better to split off the changes to the LSP into a separate PR. Unless completion is dependent upon them?

Philipp-M · 2022-05-30T07:04:30Z

You mean all the commits that are also in #2507?

In case, I will rebase this as soon as the relevant commits this PR is dependent on are on master, currently only the last commit is relevant for this PR or different to #2507.

nrdxp · 2022-06-29T19:41:40Z

just fyi, tried to rebase this on master for my own use and even though there was only a minor merge conflict in the languages.toml, it doesn't actually build after the changes made in #2738

If trying to use your new macro where the old language_server! was called I get a type error:

error[E0277]: `(dyn for<'r, 's, 't0> FnOnce(&'r mut compositor::Compositor, &'s mut compositor::Context<'t0>) + 'static)` cannot be sent between threads safely
   --> helix-term/src/commands/lsp.rs:848:8
    |
848 |     cx.callback(
    |        ^^^^^^^^ `(dyn for<'r, 's, 't0> FnOnce(&'r mut compositor::Compositor, &'s mut compositor::Context<'t0>) + 'static)` cannot be sent between threads safely

Philipp-M · 2022-06-29T20:26:19Z

Rebased to current master and fixed the issue (the acquire for the language-server inside the closure was unnecessary as only the offset-encoding is required there)

nrdxp · 2022-07-09T20:18:36Z

What is left to move this out of draft status? I've been using this for over a week now and it seems to work fine.

Philipp-M · 2022-07-09T20:54:11Z

Well for one (biggest blocker): This is currently dependent on a few commits (particularly the extension/merging of the completion menu) of #2507, and I'm unsure how to progress as I'm awaiting some feedback there.

Also support for windows paths should be added, this should be simple (by extending the path regex), but I would like to first resolve the first issue.

Another (smaller, maybe not worth to fix) issue is, that currently only one path per line is possible, this could probably be tackled with a different path-regex. But I've read that every character but \0 is allowed in a unix filepath, so I think it's difficult to find a good regex that doesn't have (realworld) limitations.

I've also experimented with async io (via tokio::fs), but IMHO the performance-drop and probably race-conditions (in the completion menu, if writing to fast/slow IO) wasn't worth it to continue that path (yet).

cole-h · 2022-11-17T02:48:26Z

helix-term/src/commands.rs

+ let items = ui::PATH_REGEX
+ .find(&line_until_cursor)
+ .and_then(|matched_path| {
+ let matched_path = matched_path.as_str();


Would it be possible to also add a denylist setting? I run NixOS, so attempting to index /nix/store would be extremely slow with its (currently) ~265000 files / directories.

Maybe we can just use the .(git)ignore for that?

Actually I've used nix/store for stress testing/general feeling, and I thought it was acceptable given the amount of files (but I've got a fast CPU and NVME drive for /nix ...))

Would someone actually edit something directly in /nix/store or in a directory with that much files/folders?

I'm not sure of a daily usecase currently where this would really be a problem, but I'm open for different solutions (my suggestion would be a global .ignore)

I don't edit things in the Nix store, no, but I do use Helix to read files in there very frequently. I'd rather not use a .ignore for that because so many other tools read that, and I'd like to set this only in Helix.

Right now it's less of a blocker (literally :)) for typing, as it's now async and results will be discarded when not relevant anymore (i.e. the user has typed something that doesn't match the completion item).

On a fast computer it's still acceptable IMHO, especially if the folder was indexed once (i.e. is likely cached in memory when accessing again)).

If this is still relevant, can you/someone maybe think/prototype how the UX/configuration for this look like (I'm happy to implement it, but I'm not sure how the UI for this should look like).

But the (completion-)menu is quite slow with > ~100k entries that are sorted synchronously every keystroke.
I think a more sophisticated lazier/async/"streamable" menu/picker implementation would be probably help in general (I still think that the behavior is so similar (also in the future), that the implementations should be shared, but that's a different topic...).

nrdxp · 2023-01-26T20:04:35Z

Did something change? After rebasing on the last push path completion seems to have just stopped working all together. Do I need to configure something differently?

Philipp-M · 2023-01-26T20:36:42Z

Have you tried using the HEAD of this branch?

My personal fork (rebased on this branch) is still working.

It's a little bit out of sync with master, I will try to find time soon to rebase this and the multiple-language-servers PR onto master.

nrdxp · 2023-01-27T03:42:06Z

Oops, I figured it out. It was my own fault, I forgot to push my latest changes of my forked branch (based on this branch) to the remote so my system never pulled my rebase. Apologies 🙏

…ith multiple different item sources (async, sync, refetch) This PR unifies the functionality of the `Picker` and the `Menu` in the struct `OptionsManager` (i.e. matching/filtering, sorting, cursor management etc.), and extends the logic of `score` by being able to retain the cursor position (which is useful, if new entries are added async). The main reason for this PR though is to have multiple option `ItemSource`s, which can be async and which may be refetched (see helix-editor#5055) to provide the options for the `OptionsManager`. It currently allows 3 different input sources: synchronous data, asynchronous data, and asynchronous functions which provide the data, which are reexecuted after an `IdleTimeout` occured (similar as helix-editor#5055). This PR is or will be a requirement for helix-editor#2507 (currently provided by helix-editor#3082), which I will rebase onto this PR soon. Noticeable effects right now: * The `Menu` now has the more sophisticated logic of the picker (mostly better caching while changing the query) * The `Menu` currently *doesn't* reset the cursor position, if `score()` is called (effectively different pattern query), I'm not sure if we should keep this behavior, I guess it needs some testing and feedback. I have kept the original behavior of the Picker (everytime `score()` changes it resets the cursor) This PR is an alternative to helix-editor#3082, which I wasn't completely happy with (adding options async was/is kind of a hack to keep it simple) thus this PR will supersede it. It was originally motivated by helix-editor#5055 and how to integrate that logic into helix-editor#2507 (which is not done yet). The PR unfortunately grew a little bit more than I've anticipated, but in general I think it's the cleaner and more future proof way to be able to have async item sources, which may change the options overtime. For example the same logic of helix-editor#5055 could be simply implemented for the completion menu with different multiple completion sources, whether it makes sense or not (though also one of the motivations of this PR to have this possibility). This should also cleanup and greatly (hopefully) simplify helix-editor#2507. To make reviewing easier (I hope) I provide a technical summary of this PR: * `OptionsManager` contains all logic that is necessary to manage options (d'oh), that means: * Filtering/matching of options with `score()` which takes a few more parameters now: * *Optional* pattern to be able to recalculate the score without providing a pattern (necessary for async addition of the options) * Boolean flag to retain the cursor, since it will likely be annoying, if an item source takes so long that the user is already selecting options of a different item source and it gets resetted because score has to be called again for matches that respect the new option source). * force_recalculation, kind of an optimization, if nothing external has changed (except new async options) * The `OptionsManager` is fed by `ItemSource`s * Fetching of items from the item source is started by the creation of the `OptionsManager` (`create_from_item_sources`), and as soon one item source provides some items a construction callback is executed (see below) * Async fetching is done by looping over `FuturesUnordered` async requests and adding the options via an `mpsc`, I wanted to avoid any `Mutex` or something alike to keep the rest of the logic as is (and likely to be more performant as well). Thus instead the editor `redraw_handle` is notified when new options are available and in the render functions new options are polled (via `poll_for_new_options`). Although not optimal, it was IMHO the best tradeoff (I tried different solutions, and came to that conclusion). * Options are stored with an additional index to the item source to be able to easily change the options on the fly, it provides some iterator wrapper functions to access the options/matches outside as usual. * When using the `OptionManager` with async sources, I have decided (to mostly mirror current logic) to use a "constructor callback" in the `create_from_item_sources` for creating e.g. the actual `Menu` or `Picker` when at least one item is available (this callback is only then executed), otherwise an optional other callback can show e.g. editor status messages, when there was no item source provided any data. * The `(File-)Picker` and `Menu` now takes the `OptionsManager` instead of the items or editor data (this is now provided in the `ItemSource`) * For sync data it's also possible to directly create the `OptionsManager` without a callback using `create_from_items` but only with this function. * I've imported the `CompletionItem` from helix-editor#2507 to provide additional information where the item came from (which language server id and offset encoding) when multiple language servers are used as Item source, it's currently an enum to avoid unnecessary refactoring if other completion sources are added (such as helix-editor#2608 or helix-editor#1015), though it's not really relevant for this PR currently (as the doc has just one language server anyway), but I think it may be easier to review it here separated from helix-editor#2507 and it's also the (only) dependency of helix-editor#2507 for helix-editor#2608 * The `DynamicPicker` is removed as this behavior can now be modeled with a `ItemSource::AsyncRefetchOnIdleTimeoutWithPattern` which is done for the `workspace_symbol_picker`

danielgomez3 · 2023-05-03T19:32:45Z

Any status on when this might be merged to main?

Philipp-M · 2023-05-03T20:07:54Z

This is not yet ready to merge and needs a major rebase/refactor and support for windows. I'll wait for this at least until multiple language servers support is merged.
I'm also not completely sure if it makes sense to put this into core (could be a super simple language server, that can be enabled for all languages).

nrdxp · 2023-05-20T03:39:09Z

Now that multi-language support is merged we should either close this or rebase and merge. I would vote for the later since I think a consistent path completion baked right into the editor makes sense.

Philipp-M · 2023-06-12T13:55:50Z

I have now rebased (or more like rewritten) it onto master.

It now uses the existing lsp::CompletionItem and adds a new field enum field source to ui::CompletionItem (that might be extended when more completion sources will be implemented, instead of previously a custom type.
I think this is a little bit more portable (could even be used with little modification for a dedicated language server) and requires fewer changes in completion.rs. It's generally a cleaner and improved implementation, and should be easier to maintain/review IMHO.

Following is new:

documentation preview (file type, file permissions, full canonicalized path).
home-path resolution (~/path)
async (via spawn_blocking instead of tokios file accessor, as they IMHO make the code less readable and are quite a bit slower than just spawning a "thread")
per-language overridable path-completion support

I think it's now in a good state for review/merging for unix platforms at least, but windows support is still missing, I'm not sure if it should be merged without windows path support...?

I don't have access to a windows machine currently and would like to avoid setting up a VM, maybe someone else can help out here, via a PR to my branch path-completion? Maybe if I find time for it (I have a few other ongoing projects, and it's not a high priority for me), I'll write some integration tests for windows paths (and for unix...). Help/support here is also appreciated.

Philipp-M · 2023-06-12T14:29:49Z

The tests for OSX failed previously (now they run through, pushed a fix), due to libc::mode_t being 16 bit on OSX (is this correct?). Since this is only tested on linux, maybe someone with OSX should test this (which I also don't have access to currently). This is relevant for the file permissions in the documentation window, which should look something like [rwxr-xr--].

inalone · 2023-08-10T19:14:03Z

Would love for this to be merged, this is the main feature I'm missing from my neovim config.

nrdxp · 2023-09-27T20:53:08Z

just an FYI, since this is the only thing stopping me from tracking the latest master, I went ahead and worked out the merge conflicts myself, in this branch:
https://github.com/nrdxp/helix/tree/fork3

Feel free to steal it to quickly fix the conflicts in this PR branch

Philipp-M · 2023-09-28T12:22:15Z

Thanks for doing this! I have rebased this on master as well (merge conflicts are fortunately maintainable).

Well as said, feedback for OSX especially would be appreciated. Whether Windows needs to be supported within this PR as well (but my feeling is, that it should be, but I'm not super keen in adding it, I'd appreciate a PR on top of this PR).

One thing I noticed after implementing this, is that there's similar code already within helix_term::ui::completers it's slightly different though, so it may or may not be worth it to share the code between (in-editor-view) completion sources...

Advantage for the current code is, that it's quite portable though.

FWIW I'm maintaining my own fork with the new event-system, rainbow-treesitter matches, and a few other smaller fixes (possibly necessary due to merges) in https://github.com/Philipp-M/helix/tree/personal/

nilsherzig · 2023-10-18T21:06:52Z

Nice work :)

Do you think there would be any problems if the autocompletion would add a '/' at the end of a directory name? This would enable a much smoother "autocompletion chain" for files deep inside some nested folders.

eliliam · 2024-01-02T18:50:49Z

Once we get that linting error fixed, is this good to go? I'd love to see this merged in since it seems like a really great implementation of path autocomplete!

sn0n · 2024-01-28T00:14:41Z

any updates? I think this is the functionality I'm looking for,.. new to helix and figure out gf for goto file, but can't figure out the 'insert file name here from this list' type path completion. sorry to bother.

daedroza · 2024-02-25T02:46:22Z

@Philipp-M : Hey, can this PR be updated with newest HEAD? The place where we hook isn't commands anymore but in separate completion handler.

* Autocompletion is triggered with `/`. * Documentation preview (file type, file permissions, canonicalized full path). * Home-path resolution (`~/path`) * Link resolution (makes sense for preview, since the LSP specification (`CompletionItemKind`) only supports files and folders but not symlinks) * Async (via `spawn_blocking` instead of tokios file accessor functions, as they IMHO make the code less readable and are quite a bit slower than just spawning a "thread") * Configurable with `editor.path-completion` (default `true`), per-language overrideable path-completion support * Adds a new enum field `source` to `ui::CompletionItem` (that might be extended, with more completion sources)

Philipp-M · 2024-02-25T18:21:45Z

Rebased onto master.

if the autocompletion would add a '/' at the end of a directory name?

I guess that could be done, possibly configurable, I like the quick retrigger via / though (while not triggering a new directory while selecting the item in the menu). Not sure about it...

any updates?

Well as described in my last comment I'm awaiting general feedback.

ddogfoodd · 2024-04-16T16:13:14Z

@Philipp-M thanks for the work. I built https://github.com/Philipp-M/helix/tree/path-completion/ on MacOS 14.4.1 and path completion worked for me as expected.

Let me know if you want specific things tested.

Philipp-M force-pushed the path-completion branch from e325640 to d4dfb7d Compare June 4, 2022 11:15

Philipp-M force-pushed the path-completion branch from d4dfb7d to bc62cb1 Compare June 21, 2022 20:19

the-mikedavis linked an issue Jun 25, 2022 that may be closed by this pull request

autocompleting file paths #2874

Open

Philipp-M force-pushed the path-completion branch from bc62cb1 to 0865642 Compare June 29, 2022 20:23

Philipp-M force-pushed the path-completion branch from 0865642 to f5ae055 Compare July 7, 2022 11:21

Philipp-M force-pushed the path-completion branch from f5ae055 to 344f3d4 Compare July 27, 2022 10:55

Philipp-M force-pushed the path-completion branch from 344f3d4 to 24dff03 Compare August 11, 2022 19:36

Philipp-M force-pushed the path-completion branch from 24dff03 to 354ac33 Compare September 7, 2022 12:42

kirawi added A-helix-term Area: Helix term improvements S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. labels Sep 13, 2022

Philipp-M force-pushed the path-completion branch 2 times, most recently from e224b0d to 1dc2381 Compare September 26, 2022 21:32

Philipp-M force-pushed the path-completion branch from 1dc2381 to 3721d63 Compare October 16, 2022 15:19

Philipp-M force-pushed the path-completion branch from 3721d63 to 9193aa4 Compare October 27, 2022 23:11

Philipp-M force-pushed the path-completion branch from 9193aa4 to 0af6cfe Compare November 5, 2022 23:48

cole-h reviewed Nov 17, 2022

View reviewed changes

Philipp-M force-pushed the path-completion branch 3 times, most recently from 7435f08 to b594f49 Compare November 25, 2022 00:16

Philipp-M force-pushed the path-completion branch from b594f49 to 12c0889 Compare December 8, 2022 22:45

Philipp-M mentioned this pull request Feb 14, 2023

Refactor picker/menu logic into an options manager which can be fed with multiple different item sources (async, sync, refetch) #5991

Closed

the-mikedavis mentioned this pull request Apr 13, 2023

File (and directory) name completion #6745

Closed

Philipp-M mentioned this pull request Jun 12, 2023

Fix underflow when repeating a completion that has a negative shift position #7322

Merged

Philipp-M force-pushed the path-completion branch from 12c0889 to 2e284aa Compare June 12, 2023 13:55

Philipp-M marked this pull request as ready for review June 12, 2023 13:57

the-mikedavis added S-waiting-on-review Status: Awaiting review from a maintainer. and removed S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. labels Jun 12, 2023

Philipp-M force-pushed the path-completion branch from 2e284aa to c239f8d Compare June 12, 2023 14:12

Philipp-M force-pushed the path-completion branch from c239f8d to e9da66c Compare September 28, 2023 12:21

the-mikedavis mentioned this pull request Nov 23, 2023

Completion with local filenames #8893

Closed

archseer added this to the next milestone Jan 28, 2024

Philipp-M force-pushed the path-completion branch from e9da66c to 3e4011f Compare February 25, 2024 18:21

estin mentioned this pull request Mar 11, 2024

Add paths autocompletion estin/simple-completion-language-server#38

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for path completion #2608

Add support for path completion #2608

Philipp-M commented May 29, 2022 •

edited

Loading

kirawi commented May 30, 2022

Philipp-M commented May 30, 2022

nrdxp commented Jun 29, 2022

Philipp-M commented Jun 29, 2022

nrdxp commented Jul 9, 2022

Philipp-M commented Jul 9, 2022

cole-h Nov 17, 2022 •

edited

Loading

Philipp-M Nov 25, 2022

cole-h Nov 25, 2022

Philipp-M Jun 12, 2023

nrdxp commented Jan 26, 2023

Philipp-M commented Jan 26, 2023

nrdxp commented Jan 27, 2023

danielgomez3 commented May 3, 2023

Philipp-M commented May 3, 2023

nrdxp commented May 20, 2023

Philipp-M commented Jun 12, 2023

Philipp-M commented Jun 12, 2023

inalone commented Aug 10, 2023

nrdxp commented Sep 27, 2023 •

edited

Loading

Philipp-M commented Sep 28, 2023

nilsherzig commented Oct 18, 2023

eliliam commented Jan 2, 2024

sn0n commented Jan 28, 2024 •

edited

Loading

daedroza commented Feb 25, 2024

Philipp-M commented Feb 25, 2024

ddogfoodd commented Apr 16, 2024 •

edited

Loading

Add support for path completion #2608

Are you sure you want to change the base?

Add support for path completion #2608

Conversation

Philipp-M commented May 29, 2022 • edited Loading

kirawi commented May 30, 2022

Philipp-M commented May 30, 2022

nrdxp commented Jun 29, 2022

Philipp-M commented Jun 29, 2022

nrdxp commented Jul 9, 2022

Philipp-M commented Jul 9, 2022

cole-h Nov 17, 2022 • edited Loading

Choose a reason for hiding this comment

Philipp-M Nov 25, 2022

Choose a reason for hiding this comment

cole-h Nov 25, 2022

Choose a reason for hiding this comment

Philipp-M Jun 12, 2023

Choose a reason for hiding this comment

nrdxp commented Jan 26, 2023

Philipp-M commented Jan 26, 2023

nrdxp commented Jan 27, 2023

danielgomez3 commented May 3, 2023

Philipp-M commented May 3, 2023

nrdxp commented May 20, 2023

Philipp-M commented Jun 12, 2023

Philipp-M commented Jun 12, 2023

inalone commented Aug 10, 2023

nrdxp commented Sep 27, 2023 • edited Loading

Philipp-M commented Sep 28, 2023

nilsherzig commented Oct 18, 2023

eliliam commented Jan 2, 2024

sn0n commented Jan 28, 2024 • edited Loading

daedroza commented Feb 25, 2024

Philipp-M commented Feb 25, 2024

ddogfoodd commented Apr 16, 2024 • edited Loading

Philipp-M commented May 29, 2022 •

edited

Loading

cole-h Nov 17, 2022 •

edited

Loading

nrdxp commented Sep 27, 2023 •

edited

Loading

sn0n commented Jan 28, 2024 •

edited

Loading

ddogfoodd commented Apr 16, 2024 •

edited

Loading