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

Tab completion of paths prefixed by ~ does not work #4061

Closed
ixjlyons opened this Issue May 24, 2017 · 8 comments

Comments

Projects
None yet
4 participants
@ixjlyons

ixjlyons commented May 24, 2017

cd /home/kenny/sr<TAB> correctly completes to cd /home/kenny/src whereas cd ~/sr<TAB> does not (though I can press the right arrow key to complete). I ran a bisect which identified a3a0692 as the offending commit.

@krader1961 krader1961 added this to the fish-future milestone May 24, 2017

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 May 24, 2017

Contributor

Good catch. Thanks for the bug report. This is an example of why we didn't want to include that change in the forthcoming 2.6.0 release. It wasn't clear that change wouldn't have negative side-effects.

P.S., Pressing [right-arrow] is accepting the autosuggestion. Something that is independent of file name completions.

Contributor

krader1961 commented May 24, 2017

Good catch. Thanks for the bug report. This is an example of why we didn't want to include that change in the forthcoming 2.6.0 release. It wasn't clear that change wouldn't have negative side-effects.

P.S., Pressing [right-arrow] is accepting the autosuggestion. Something that is independent of file name completions.

@zx8

This comment has been minimized.

Show comment
Hide comment
@zx8

zx8 May 26, 2017

In addition, this seems to have broken "partial matching" of directories as well, i.e.

Expected

$ ls
aaaabbbb  bbbbaaa  cccccddddd
$ cd d<TAB>
$ cd cccccddddd/

Actual

$ ls
aaaabbbb  bbbbaaa  cccccddddd
$ cd d<TAB>
[no results]

zx8 commented May 26, 2017

In addition, this seems to have broken "partial matching" of directories as well, i.e.

Expected

$ ls
aaaabbbb  bbbbaaa  cccccddddd
$ cd d<TAB>
$ cd cccccddddd/

Actual

$ ls
aaaabbbb  bbbbaaa  cccccddddd
$ cd d<TAB>
[no results]
@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho May 26, 2017

Member

The issue here is that cd is using two kinds of completions - the script, and some hardcoded ones in c++. These latter ones are classified as "file" completions.

The script does essentially one thing the hardcoded completions don't - for candidates from $CDPATH, it adds the corresponding $CDPATH-component as a description (which I find rather useful). On the other hand, it does not do various expansions, as you've seen here.

There are a few things we can do:

  • Remove the cd.fish completion script since it's barely useful anyway - we can add the descriptions back later

  • Add these expansions to the cd.fish script (and then remove the special expansion code). This is difficult to do since we'd have to redo a bunch of the matching logic, and then keep it in sync.

  • Remove the "-x" from cd.fish - this makes it work immediately, and is exactly as clean as the status quo.

Since the third option is really easy to do and fixes everything now, I'm going to do that.


The good news is that we now know what to look out for when testing the underlying change - anything that might use the file-matching logic as a fallback for missing expansions, which includes everything using __fish_complete_directories.

Most other completions won't be affected since they either use the builtin file-completion logic or don't accept files at all.

Member

faho commented May 26, 2017

The issue here is that cd is using two kinds of completions - the script, and some hardcoded ones in c++. These latter ones are classified as "file" completions.

The script does essentially one thing the hardcoded completions don't - for candidates from $CDPATH, it adds the corresponding $CDPATH-component as a description (which I find rather useful). On the other hand, it does not do various expansions, as you've seen here.

There are a few things we can do:

  • Remove the cd.fish completion script since it's barely useful anyway - we can add the descriptions back later

  • Add these expansions to the cd.fish script (and then remove the special expansion code). This is difficult to do since we'd have to redo a bunch of the matching logic, and then keep it in sync.

  • Remove the "-x" from cd.fish - this makes it work immediately, and is exactly as clean as the status quo.

Since the third option is really easy to do and fixes everything now, I'm going to do that.


The good news is that we now know what to look out for when testing the underlying change - anything that might use the file-matching logic as a fallback for missing expansions, which includes everything using __fish_complete_directories.

Most other completions won't be affected since they either use the builtin file-completion logic or don't accept files at all.

@faho faho modified the milestones: fish 2.7.0, fish-future May 26, 2017

@faho faho closed this in 4afd418 May 26, 2017

@zx8

This comment has been minimized.

Show comment
Hide comment
@zx8

zx8 May 27, 2017

@faho 4afd418 seems to have fixed the "partial completion" issue and the cd ~/<TAB> cases but cd ~<TAB> (without the / after ~) still seems to be broken (i.e. on my mac, it somehow autocompletes to cd ~_amavisd). Previously, it would autocomplete all the directories within ~.

zx8 commented May 27, 2017

@faho 4afd418 seems to have fixed the "partial completion" issue and the cd ~/<TAB> cases but cd ~<TAB> (without the / after ~) still seems to be broken (i.e. on my mac, it somehow autocompletes to cd ~_amavisd). Previously, it would autocomplete all the directories within ~.

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho May 27, 2017

Member

cd ~ (without the / after ~) still seems to be broken (i.e. on my mac, it somehow autocompletes to cd ~_amavisd)

That means you have a user called "_amavisd" - tilde-expansion covers not just ~/ for the current user's home, but also ~$USER/ for $USER's home. E.g. on my system ~root/ expands to "/root/".

This isn't directly caused by the change to not offer files. It is caused by c114cbc since that breaks on the first match.

@krader1961: Mind taking a look?

Member

faho commented May 27, 2017

cd ~ (without the / after ~) still seems to be broken (i.e. on my mac, it somehow autocompletes to cd ~_amavisd)

That means you have a user called "_amavisd" - tilde-expansion covers not just ~/ for the current user's home, but also ~$USER/ for $USER's home. E.g. on my system ~root/ expands to "/root/".

This isn't directly caused by the change to not offer files. It is caused by c114cbc since that breaks on the first match.

@krader1961: Mind taking a look?

@zx8

This comment has been minimized.

Show comment
Hide comment
@zx8

zx8 May 27, 2017

@faho Aha, I see. I'm assuming also related to c114cbc, it seems to complete ~_amavisd even if the target directory doesn't exist:

$ grep _amavisd /etc/passwd
_amavisd:*:83:83:AMaViS Daemon:/var/virusmails:/usr/bin/false

$ cd ~_amavisd
cd: The directory '/private/var/virusmails' does not exist

zx8 commented May 27, 2017

@faho Aha, I see. I'm assuming also related to c114cbc, it seems to complete ~_amavisd even if the target directory doesn't exist:

$ grep _amavisd /etc/passwd
_amavisd:*:83:83:AMaViS Daemon:/var/virusmails:/usr/bin/false

$ cd ~_amavisd
cd: The directory '/private/var/virusmails' does not exist
@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho May 27, 2017

Member

it seems to complete ~_amavisd even if the target directory doesn't exist

I'm unsure if we should do anything about that. That is what the "_amavisd" user's $HOME is set to, and it might be useful for some commands. I'm assuming the path is still marked as non-existing (i.e. it's not underlined)?

Anyway, I've now opened #4074 to track the ~/ issue (which I don't was ever offered as a completion).

Member

faho commented May 27, 2017

it seems to complete ~_amavisd even if the target directory doesn't exist

I'm unsure if we should do anything about that. That is what the "_amavisd" user's $HOME is set to, and it might be useful for some commands. I'm assuming the path is still marked as non-existing (i.e. it's not underlined)?

Anyway, I've now opened #4074 to track the ~/ issue (which I don't was ever offered as a completion).

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho May 27, 2017

Member

Also #4075 tracks the issue of only completing one user.

Member

faho commented May 27, 2017

Also #4075 tracks the issue of only completing one user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment