MilesCranmer / rip2

A safe and ergonomic alternative to rm
GNU General Public License v3.0
26 stars 1 forks source link

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

Closed Mawdac closed 11 hours ago

Mawdac commented 5 days ago

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 commented 5 days ago

Thanks for the report!

For reference, the completions get generated by clap_complete here: https://github.com/MilesCranmer/rip2/blob/75ea37ef4ea16ae64809e9c50999d95fd6a809bd/src/completions.rs#L24

I think I'm specifying subcommands correctly here: https://github.com/MilesCranmer/rip2/blob/75ea37ef4ea16ae64809e9c50999d95fd6a809bd/src/args.rs#L114.

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 commented 5 days ago

Some related issues:

MilesCranmer commented 5 days ago

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).

Mawdac commented 4 days ago

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 commented 4 days ago

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!

Mawdac commented 4 days ago

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 commented 4 days ago

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

Mawdac commented 4 days ago

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

tesuji commented 3 days ago

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 commented 3 days ago

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)

Mawdac commented 1 day ago

Thanks for the upstream fix @tesuji!

MilesCranmer commented 2 hours ago

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