Skip to content

Conversation

@cyqsimon
Copy link
Contributor

See #2085 (comment).

TLDR: autocompleting the cache subcommand causes usability issues. This PR disables it.

I am not terribly familiar with completion scripts TBH. If you are more competent please help check my edits, thanks.

Copy link
Collaborator

@Enselic Enselic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm no shell completion expert, but these changes look reasonable to me. And I am strongly in favor of the what the PR wants to achieve.

I'll set this as Approved. I'll give some more time for people to look at this. If no more input comes in, I'll merge this.

I think it is better to merge it so that problems can be found, if any, rather than letting the PR sit untouched for much longer.

Ideally we would have regression tests for completion scripts, but life is not always ideal.

@Enselic
Copy link
Collaborator

Enselic commented Dec 11, 2022

@cyqsimon Can you also add an entry to CHANGELOG.md please?

@sharkdp
Copy link
Owner

sharkdp commented Dec 11, 2022

I agree. Thank you @cyqsimon. I would appreciate if we could remove the code in question, instead of commenting it. We would still keep the comments explaining why we're not completing cache, of course.

@cyqsimon
Copy link
Contributor Author

@Enselic @sharkdp all done.

@Enselic Enselic merged commit b6b9d3a into sharkdp:master Dec 18, 2022
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 this pull request may close these issues.

3 participants