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

Fish completions don't include filepaths unless a subcommand is first given #39

Closed
maddawik opened this issue Jun 25, 2024 · 12 comments · Fixed by #40
Closed

Fish completions don't include filepaths unless a subcommand is first given #39

maddawik opened this issue Jun 25, 2024 · 12 comments · Fixed by #40

Comments

@maddawik
Copy link

maddawik commented Jun 25, 2024

The completions for fish (haven't tested any others yet) only allow you to complete subcommands or options and not files/folders without first specifying a subcommand. This is pretty inconvenient for general rip usage

Ex.

touch mini
rip mi # tab to complete suggests `completions` instead of `mini`
rip help # tab after this and you get files/folders

I haven't dug into the completions but I imagine it would be a little tricky given they're generated from another lib it appears, haven't looked too deep - just wanted to report it. I get around it using fzf to pick things.

Also thanks for bringing rip back to life

@MilesCranmer
Copy link
Owner

Thanks for the report!

For reference, the completions get generated by clap_complete here:

generate(tryshell.unwrap(), &mut args::Args::command(), "rip", buf);

I think I'm specifying subcommands correctly here:

#[derive(Subcommand, Debug)]
.

When I try it on my machine with nushell, it seems to work correctly:

touch mini
# rip mi <tab>
rip mini

So could be an issue in the clap completions generated for fish?

@MilesCranmer
Copy link
Owner

Some related issues:

@MilesCranmer
Copy link
Owner

Might be worth reporting it in the clap repo? I'm not sure if they are aware of this issue. But I don't think I can fix on rip side, it will need to be fixed upstream (unless I'm using clap complete wrong).

@maddawik
Copy link
Author

maddawik commented Jun 26, 2024

I had some time to play around with this, the completion that's generated includes -f (or --no-files, preventing file completion) on the 3 subcommands

complete -c rip -n __fish_use_subcommand -f -a completions -d 'Generate shell completions file'
complete -c rip -n __fish_use_subcommand -f -a graveyard -d 'Print the graveyard path'
complete -c rip -n __fish_use_subcommand -f -a help -d 'Print this message or the help of the given subcommand(s)'

Removing those solves it, though like you said this is clearly upstream - I am just not confident enough right this moment to open an issue in that repo since I don't really know rust (and there looks to be a lot of potentially related fish issues). But I understand if you want to close this, I made a temp fix https://github.com/mawdac/fish-rip for anyone else who wants a temp workaround. Thanks for the feedback!

@MilesCranmer
Copy link
Owner

MilesCranmer commented Jun 26, 2024

Awesome!

I think it would be very appreciated by the clap team if you post an issue pointing it out. Even if someone has already posted a related issue, better safe than sorry :) If I was the maintainer I would definitely want to know about this!

@maddawik
Copy link
Author

maddawik commented Jun 26, 2024

I agree with you wholeheartedly, I just don't think I can provide them the info they need for investigating this without just pointing to this issue (which they discourage in their issue template) - i.e., it's a skill issue on my end lol

Screenshot 2024-06-26 at 8 29 26 AM

@MilesCranmer
Copy link
Owner

MilesCranmer commented Jun 26, 2024

You can always post a blank issue: https://github.com/clap-rs/clap/issues/new (as a maintainer myself, sometimes even if people post an issue without any details, it can make me realise something about the code that is buggy)

Screenshot 2024-06-26 at 13 38 00

I think you can just say exactly what you told me and copy-paste my responses and link this thread

@maddawik
Copy link
Author

Good call, submitted! Feel free to do what you'd like with this issue 😄 thanks again for the help!

@tesuji
Copy link

tesuji commented Jun 27, 2024

I'm agree that the generated completion should not ignore files. But if there is a file name graveyard just when calling rip2 graveyard, what would rip2 do?

@MilesCranmer
Copy link
Owner

But if there is a file name graveyard just when calling rip2 graveyard, what would rip2 do?

Good question. Maybe we could have an error message that the user should write it as ./graveyard due to an ambiguous file?

I think most of the time people just don't worry about this since it's rare enough. For example, you can technically ./-h as a filepath. In which case most commands with a help flag would face the same question.

(By the way, rip is the binary name rather than rip2)

@maddawik
Copy link
Author

Thanks for the upstream fix @tesuji!

@MilesCranmer
Copy link
Owner

The release v0.8.1 is built with the latest clap. Thanks @tesuji!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants