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

better fish completions #2275

Merged
merged 4 commits into from Aug 27, 2022
Merged

better fish completions #2275

merged 4 commits into from Aug 27, 2022

Conversation

zgracem
Copy link
Contributor

@zgracem zgracem commented Aug 16, 2022

Back in 2020, the command-line completions for bat that had been shipping with fish were removed in favour of the completions file that currently ships with bat (see discussion in #651). Unfortunately, the deprecated fish completions had some nifty features that weren't carried over. This PR restores those features and expands upon them.

Improvements vs. the current completions:

  • Less verbose descriptions → better columned layout.
    • Notably, default options (e.g. always, auto) are indicated in the option-completion text, not the description of the option itself.
  • Supports all documented options, including those for bat cache.
  • Fully (!) supports completing bat --map-syntax <glob pattern>:<language syntax>.
  • Supports multiple comma-separated arguments to bat --style.
  • Regular output options (e.g. --style) won't be suggested if a previous option (e.g. --help) overrides regular output/behaviour.
  • The {{PROJECT_EXECUTABLE}} placeholders have been place-held by the locally-scoped $bat, for ease of reading and editing.
    • The names of the support functions (e.g. __bat_complete_map_syntax) are no longer parameterized, but "the other bat" doesn't include shell completions as far as I can tell, so there shouldn't be any collisions.

Please let me know if you have any questions!


Before:

bat-before

After:

bat-after

bat --italic-text= before:

bat-italic-before

bat --italic-text= after:

bat-italic-after

bat --style= before:

bat-style-before

bat --style= after:

bat-style-after

This will cause vim (and other properly-extended editors) to read this
as a fish script file, despite its .fish.in extension.
@Enselic
Copy link
Collaborator

Enselic commented Aug 25, 2022

Looks reasonable, so I am tempted to blindly merge this.

Ideally, all important aspects of fish completions should have automated tests, because manually testing stuff is not manageable in the long term. Until we have tests, I think blindly merging stuff that looks reasonable is not unreasonable.

I'll leave this comment here for a while to give other maintainers a chance to object if they want.

@keith-hall You seemed to enjoy the change, so what do you think about blindly merging?

We should add an entry to CHANGELOG.md though.

@keith-hall
Copy link
Collaborator

I had the same thoughts regarding automated tests, but this change is described well enough (and looks manually tested enough by the author) that I'd be okay with blindly merging it. Worst case outcome, if something breaks, it's only fish completions ;)

@Enselic
Copy link
Collaborator

Enselic commented Aug 26, 2022

Sounds good, thanks for the input!

@zgracem Can you add an entry to CHANGELOG.md please? After that we can merge this PR

@zgracem
Copy link
Contributor Author

zgracem commented Aug 27, 2022

Can you add an entry to CHANGELOG.md please?

All done!

@Enselic
Copy link
Collaborator

Enselic commented Aug 27, 2022

Thank you!

@Enselic Enselic merged commit a6297b2 into sharkdp:master Aug 27, 2022
@zgracem zgracem deleted the better-fish-completions branch August 27, 2022 16:35
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.

None yet

4 participants