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

Generate fish completions #64

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Generate fish completions #64

wants to merge 1 commit into from

Conversation

coszio
Copy link
Contributor

@coszio coszio commented Mar 7, 2025

A bit of UX improvement,

  • installs bfb fish completions in ~/.config/fish/completions
  • adds "suggestions" feature to clap to get nice suggestions on typos

@coszio coszio requested review from timvisee and generall March 7, 2025 21:33

include!("src/args.rs");

fn main() -> Result<(), Error> {
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's right for regular builds to simply dump files across your host. Ideally the build process should not touch any files outside of the projects directory to keep it isolated.

I'd prefer to create a separate script instead, which you can invoke to install the latest shell completions.

Wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think completions are nice to have than not have. There are plenty of other installations that create completions files without explicitly requesting them. From the POV of a user, less extra work when installing is a better experience.

I can think of another two solutions:

  1. Use a separate script, or bfb argument to install completions (what you mentioned)
  2. (can we set an uninstaller script?) Make sure to clean up completions on uninstall.
  3. Log a line saying that completions have been installed somewhere.

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.

2 participants