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

remove fish auto-completions from ripgrep release #1577

Closed
1 of 3 tasks
BurntSushi opened this issue May 9, 2020 · 7 comments
Closed
1 of 3 tasks

remove fish auto-completions from ripgrep release #1577

BurntSushi opened this issue May 9, 2020 · 7 comments
Labels
bug A bug.

Comments

@BurntSushi
Copy link
Owner

BurntSushi commented May 9, 2020

The main issue here is that the Fish project wants to own auto-completions for ripgrep, and generally speaking, I don't want to stop them. In particular, ripgrep's Fish auto-completions are auto-generated by Clap (ripgrep's argv parser). I have no idea how good they are, but they are only provided by ripgrep because it's easy to provide them. Otherwise, I would not maintain them.

The actual reason to drop them is that they are causing problems. See #1485 and fish-shell/fish-shell#6868 in particular.

The plan here is:

  • The ripgrep 12.1.0 release will include a notice that the Fish shell completions will be dropped from the next major version release.
  • The ripgrep 13.0.0 release will drop Fish shell completions.
  • The Fish project should be notified once the ripgrep 13 release is out so that they can merge ripgrep auto-completions as they see fit.
@BurntSushi BurntSushi added the bug A bug. label May 9, 2020
BurntSushi added a commit that referenced this issue May 9, 2020
This also removes Fish shell completions. See #1577 for more details.
@ignatenkobrain
Copy link
Contributor

@BurntSushi I am probably misunderstanding something, but how do they conflict? The fish autocompletions coming from upstream are located in /usr/share/fish/completions/ while distro maintainers are supposed to put their completions into /usr/share/fish/vendor_completions.d/ which is for example what Fedora does. The vendor ones override upstream ones.

@ignatenkobrain
Copy link
Contributor

It would be nicer if the fish completions would be maintained in upstream rather than fish upstream because that is PITA to upgrade since everyone would need to wait for release of fish to get new completions for new rg version.

@BurntSushi
Copy link
Owner Author

BurntSushi commented May 10, 2020

The vendor ones override upstream ones.

Right, that's exactly the problem AIUI. See the linked issue for discussion: fish-shell/fish-shell#6868

My perspective on this is that I'm acting on the Fish project's recommendation. So if this isn't the optimal path and you want to convince them to change their recommendation, then I'm happy to oblige.

It would be nicer if the fish completions would be maintained in upstream rather than fish upstream because that is PITA to upgrade since everyone would need to wait for release of fish to get new completions for new rg version.

I assumed the Fish project took this into account when making their recommendation? ¯\_(ツ)_/¯

I am happy to keep generating the Fish auto-completions. I don't know what their quality level is though. In theory, I'd also maybe be willing to assume maintenance of manually curated Fish auto-completions like we do for zsh, but under the following conditions:

  1. It is easy and obvious to add and remove new flags. The zsh auto-completions have extensive docs (see the bottom) to help navigate the arcane syntax.
  2. CI fails if the auto-completions are out of sync with ripgrep's flags. zsh auto-completions have this test to ensure that, for example.

cc @thomcc @faho

@faho
Copy link

faho commented May 10, 2020

I am happy to keep generating the Fish auto-completions. I don't know what their quality level is though.

They are... okay? It's not like rg needs a ton, it's not git with tens of subcommands, each accepting different types of arguments. It's mostly flags with simple "always", "never", "auto" arguments at most.

There are a few nits, like that it always checks the __fish_use_subcommand condition, which means options will only be completed before the first non-option argument. I'm not sure that's correct?

Also the description wording isn't 100% what I'd use (it is generally understandable and concise enough tho) and the option-arguments don't have descriptions.

So there isn't a whole lot to be gained by manual curation in terms of completion quality.

It is easy and obvious to add and remove new flags. The zsh auto-completions have extensive docs (see the bottom) to help navigate the arcane syntax.

Well, I mean:

complete -c rg -n "__fish_use_subcommand" -s A -l after-context -d 'Show NUM lines after each match.'
complete -c rg -n "__fish_use_subcommand" -s B -l before-context -d 'Show NUM lines before each match.'
complete -c rg -n "__fish_use_subcommand" -l color -d 'Controls when to use color.' -r -f -a "never auto always ansi"

I would imagine for simple options like here it's quite easy.

CI fails if the auto-completions are out of sync with ripgrep's flags. zsh auto-completions have this test to ensure that, for example.

If you have fish installed that's doable. Completion can be invoked from a script via complete --do-complete="the fake commandline", so complete --do-complete="rg -" would list all the options, one per line with the description after a tab.

If you don't, you'd have to parse the file yourself. In general, that's a hard problem, but you might be able to get away with it iff you required the completions to fit a very particular style.


The main issue here is that the Fish project wants to own auto-completions for ripgrep

So, the reason we merged the completions initially (which we have since backed out) was to automatically enable completions for users. The idea being that they'd install ripgrep via cargo install, which won't install the completions, and would then not think to install them themselves.

What instead ended up happening was that users used distro packages, and the distro packages already moved the completions, only to the wrong location.

And that led to ripgrep and fish not being installable together because of file conflicts and took longer than I would have liked to sort out.

Now a few months have passed, and I have since removed the ripgrep completions from fish again, mostly so we wouldn't run into the installability issue again. Distros have also changed to using the correct path in the meantime.

I assumed the Fish project took this into account when making their recommendation? ¯_(ツ)_/¯

You probably mean my comment "Ideally we'd just be able to coordinate with rg upstream to drop them, but I'm not entirely sure that'd work out.".

To be honest, that was in error. I was too focussed on the premise of the PR that shipping the rg completions ourselves would be good, and neglected to look up the backstory of why we shipped rg completions at all (which as it turns out wasn't related to completion quality).

Personally, I don't think the ripgrep completions gain much from being shipped in fish - they aren't super complicated so they won't benefit much from enhancements in fish, but ripgrep might add options quicker than we ship releases.

So if you are happy keeping them, then please keep them!

@BurntSushi
Copy link
Owner Author

All righty then. Happy to keep auto-generating them. It's really no extra work for me. :-)

I'll leave this issue open for a bit in case others have opinions.

@chax
Copy link

chax commented May 15, 2020

Fish developers removed rg completions from their distribution in their last release, see fish-shell/fish-shell#5822
so please don't remove it from your package.
People who package this software should take care where each file (completion or any other file) should be distributed when piece of software is packaged.
Anyways there is special dedicated folder where package's completion should go for fish (/usr/share/fish/vendor_completions.d), and special folder which is dedicated only for fish (/usr/share/fish/completions/), most packagers confuse those two and use wrong folder.

@BurntSushi
Copy link
Owner Author

I'll keep auto-generating Fish shell completions: fish-shell/fish-shell#6868 (comment)

So closing this.

BurntSushi added a commit that referenced this issue Jun 1, 2021
In a previous release, I announced that Fish completions were being
removed. But the Fish project decided to remove theirs and have
ripgrep's stay.

Closes #1577
BurntSushi added a commit that referenced this issue Jun 1, 2021
In a previous release, I announced that Fish completions were being
removed. But the Fish project decided to remove theirs and have
ripgrep's stay.

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

No branches or pull requests

4 participants