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

Insert longest common prefix #35

Closed
Dwight-D opened this issue Jun 3, 2020 · 19 comments
Closed

Insert longest common prefix #35

Dwight-D opened this issue Jun 3, 2020 · 19 comments
Labels
enhancement New feature or request

Comments

@Dwight-D
Copy link

Dwight-D commented Jun 3, 2020

Matches are presented in lexicographical order with no regard to the accuracy of the matches.

I would expect suggestions starting with my input to be prioritized over a match in a substring.

This interferes with the behavior of the tab key, as compared to in the normal shell experience. I would expect tab to first insert perfect matches starting with what I've typed, and not the first match in lexicographical order.

Steps to reproduce:

➜  cat test
atestfile  testfile
\t
➜  cat atestfile

Expected behavior:

➜  cat test
testfile  atestfile
\t
➜  cat testfile
@marlonrichert
Copy link
Owner

marlonrichert commented Jun 3, 2020

This is something I've tried to solve before, but did not commit, because it impacted performance significantly. This is because I cannot control the sorting directly. I have to use the completion system's grouping mechanism to fake it.

However, now that we have asynchronous completion, I could give it another try and at least provide it as an option, with a note about the performance hit. 🙂

@Dwight-D
Copy link
Author

Dwight-D commented Jun 3, 2020

Yeah I understand that it might be tricky but it's a fairly big annoyance since it deviates so much from normal shell behavior when hitting tab. You really expect it to complete with the right thing, so it throws me off a lot when I quickly tab and enter and it ends up inserting something completely different.

marlonrichert added a commit that referenced this issue Jun 4, 2020
@marlonrichert
Copy link
Owner

@Dwight-D Alright, here it is on the dev branch. Please try it out. 🙂

marlonrichert added a commit that referenced this issue Jun 4, 2020
marlonrichert added a commit that referenced this issue Jun 4, 2020
@Dwight-D
Copy link
Author

Dwight-D commented Jun 5, 2020

Nice! It seems to be fixed for a few issues, but the example I posted here still fails. I'll keep using it throughout the day and see if I can figure out any pattern to it.

@marlonrichert
Copy link
Owner

@Dwight-D If the example you posted here fails, then you have not enabled the feature. You should have the following in your .zshrc file, before sourcing zsh-autocomplete:

zstyle ':autocomplete:tab:*' completion insert

marlonrichert added a commit that referenced this issue Jun 5, 2020
marlonrichert added a commit that referenced this issue Jun 5, 2020
@marlonrichert marlonrichert added the enhancement New feature or request label Jun 5, 2020
@marlonrichert marlonrichert changed the title Exact matches not prioritized in suggestion ordering Insert longest common prefix Jun 5, 2020
marlonrichert added a commit that referenced this issue Jun 5, 2020
marlonrichert added a commit that referenced this issue Jun 5, 2020
marlonrichert added a commit that referenced this issue Jun 7, 2020
@Dwight-D
Copy link
Author

Dwight-D commented Jun 8, 2020

Okay, my bad. This is great, I love it!

@Dwight-D
Copy link
Author

Dwight-D commented Jun 8, 2020

No, wait, there are some weird quirks

➜  ls
bar     bartest baz
➜  cd bar
bar/      bartest/
\t
➜  cd bar/

Expected behavior: cycle through bar/ and bartest, or possibly insert bartest instead of autocompleting to bar/.

I also noticed that some autocompletions are now inserting space when I tab partway through a path.

➜  kubectl apply -f kubernetes/deploym
kubernetes/deployments/
\t
➜  kubectl apply -f kubernetes/deployments/ <--- SPACE HERE
edit-last-applied  set-last-applied   view-last-applied

Expected behavior: no space at the end of completion deployments, allow me to keep completing/cycling through file paths:

➜  kubectl apply -f kubernetes/deployments/
foo.yml bar.yml baz.yml

This last one I'm not sure if it's new behavior, can't recall when I last used kubectl like this. I haven't noticed it for any other programs yet, it doesn't happen for all completions. Might be an error in kubectl completions but it didn't use to behave like this without the plugin.

@marlonrichert
Copy link
Owner

Expected behavior: cycle through bar/ and bartest

Thanks, that's easy to change.

or possibly insert bartest instead of autocompleting to bar/.

Why? What's the logic behind that? I would expect it to complete bar/ since it's the first in the list.

Expected behavior: no space at the end of completion deployments, allow me to keep completing/cycling through file paths

I cannot reproduce that, because I don't have access to kubectl. It works for me fine with normal paths. Do you have some other way I could reproduce this?

@Dwight-D
Copy link
Author

Dwight-D commented Jun 8, 2020

Why? What's the logic behind that? I would expect it to complete bar/ since it's the first in the list.

Completing to bar/ doesn't make sense because it's not an unambiguous match, it's not a substring of bartest. However cycling through them works fine.

Honestly I couldn't remember the default shell behavior here which is why I threw in the bartest, I was thinking that maybe it kept on completing longer directory names instead of inserting the slash. But I tested it out in bash and it will apparently be considered an ambiguous match and present both options, so just forget I said that.

I cannot reproduce that, because I don't have access to kubectl. It works for me fine with normal paths. Do you have some other way I could reproduce this?

No, that's the only error I've run across so far but I'll let you know if I find any others. Most of the time it seems to work fine for me as well.

marlonrichert added a commit that referenced this issue Jun 9, 2020
marlonrichert added a commit that referenced this issue Jun 9, 2020
@marlonrichert
Copy link
Owner

@Dwight-D It's now available on the master branch.

@vendelin8
Copy link

Hi,
Awesome lib, thanks!
Anyway, I wanted to achieve common prefix as well. Generally it works fine, but when I start the command with sudo it doesn't. Root uses zsh too, with a symlink to .zshrc.

@marlonrichert marlonrichert reopened this Dec 28, 2020
@marlonrichert
Copy link
Owner

@vendelin8 It seems to work for me. Can you please give me some test cases for which it fails?

@vendelin8
Copy link

sc0
Then after Tab
sc1

My idea was sudo updatedb

@vendelin8
Copy link

My related ~/.zshrc part looks like

zstyle ':autocomplete:tab:*' insert-unambiguous yes
. /usr/share/zsh/plugins/zsh-autocomplete/zsh-autocomplete.plugin.zsh

And without sudo it works fine.

$ ls -l /root/.zshrc
lrwxrwxrwx 1 root root 18 dec   27 22.53 /root/.zshrc -> /home/panda/.zshrc

@marlonrichert
Copy link
Owner

marlonrichert commented Dec 28, 2020

My idea was sudo updatedb

And without sudo it works fine.

That is because you get more matches with sudo than without. Zsh only supports getting longest substring that is common to all completions listed. The additional completions you get when doing sudo makes it so there's no longer any substring common to all matches.

I'll see if I can make the matching a bit more strict/less fuzzy (perhaps only for users who have insert-unambiguous enabled), so that you get fewer matches, but even then, there's no guarantee that it will work for that particular case.

@vendelin8
Copy link

I'd be happy with a common prefix, if there's an option for that. Is it?

@marlonrichert
Copy link
Owner

That's the one I'm talking about. There is no difference.

However, I did some more testing and I found that there's actually a bit more going on. I'm working on a patch that will improve this.

marlonrichert added a commit that referenced this issue Dec 28, 2020
Fixes a problem mentioned in issue #35.
@marlonrichert
Copy link
Owner

@vendelin8 It should be better now. Please try again.

@vendelin8
Copy link

Awesome, it works fine, thank you!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants