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

fix __fish_list_current_token not recognizing ~ as $HOME #9954

Merged
merged 2 commits into from Aug 16, 2023
Merged

fix __fish_list_current_token not recognizing ~ as $HOME #9954

merged 2 commits into from Aug 16, 2023

Conversation

Axlefublr
Copy link
Contributor

No description provided.

@Axlefublr
Copy link
Contributor Author

it's not gonna hurt if I elaborate: I'd expect cd ~/.config to display the contents of .config, but it doesn't because it doesn't recognize ~ as home, which is incredibly annoying

I will argue that not seeing dotfiles and dotdirectories is also annoying, but I can see how not using the -A flag for the ls is a design choice. However I'm gonna jump up thrice if you agree that it's a good idea to change the ls in there to ls -A instead

@henrikhorluck
Copy link
Contributor

henrikhorluck commented Aug 14, 2023

I was wrong, I did not have this keybound I might be misunderstanding where this is used but I assume you are talking about tab-completion?

If that is the case I cannot reproduce your error, even in sh -c 'env HOME=$(mktemp -d) XDG_CONFIG_HOME= fish' (note you probably want to add another file to your temp home's .config there, only ./config/fish exists by default).

Tab-completing with cd ~/.config/ already correctly displays the contents of ~/.config/ for me on master, and it also works on 3.6.1. This works regardless of whether AppleShowAllFiles is set or not (I am on macOS)

Okay so the error is reproducible as

sh -c 'env HOME=$(mktemp -d) XDG_CONFIG_HOME= fish'
cd ~
mkdir ~/.config/test
touch hello.txt
cd ~/.config # and then enter keybind for __fish_list_current_token, which is ALT + L by default
# should print "test" and "fish", but instead prints "hello.txt"

I don't think we should special-case ~ here, $HOME/.config also does not work, it is more sensible to perform full variable-expansion akin to how tab-completion works

@Axlefublr
Copy link
Contributor Author

what's completely hilarious is that "$HOME/.config" does work, but $HOME/.config doesn't

the former being a situation you would practically never find yourself in

@Axlefublr
Copy link
Contributor Author

also yeah I agree. ideally we take the token and expand it like you would by pressing tab

maybe you're doing something like ~/.c/k intending to press tab to expand it to ~/.config/kitty

making list-current-token support that would be ideal

but honestly I don't even need ideal, I just want not ridiculous

@zanchey
Copy link
Member

zanchey commented Aug 14, 2023

The full right answer here would be #751, probably as a subcommand of path.

@faho faho added this to the fish next-3.x milestone Aug 16, 2023
@faho faho merged commit fd68aca into fish-shell:master Aug 16, 2023
5 of 7 checks passed
@faho
Copy link
Member

faho commented Aug 16, 2023

This is fine as a stop gap. Thanks, merged!

@zanchey zanchey modified the milestones: fish next-3.x, fish 3.7.0 Aug 16, 2023
@Axlefublr
Copy link
Contributor Author

Thank you for merging!

Was scared that this was going to be a situation of "we can't do it perfectly so we won't do it at all", so it's nice to see the feature being merged at least in this simplified fashion for the time being

@Axlefublr Axlefublr deleted the list-current-token branch August 17, 2023 01:53
faho pushed a commit that referenced this pull request Oct 6, 2023
* fix __fish_list_current_token not recognizing ~ as $HOME

* right. it was supposed to be $HOME. lol.

(cherry picked from commit fd68aca)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants