Skip to content

Conversation

@Ahmed-AdelB
Copy link

Summary

  • Add documentation for shell completion support via Click
  • Include setup instructions for Bash, Zsh, and Fish shells
  • Add corresponding changelog entry

Related Issue

Closes #1702

Changes

  • Added "Shell completions" section to README.md with installation instructions
  • Created changelog fragment changelog.d/+shell_completions.doc.md

Test Plan

  • Verified pip-compile generates completion script with _PIP_COMPILE_COMPLETE=bash_source
  • Verified pip-sync generates completion script with _PIP_SYNC_COMPLETE=bash_source
  • Instructions follow Click's shell completion documentation

Ahmed Adel Bakr Alderai

Copy link
Member

@sirosen sirosen left a comment

Choose a reason for hiding this comment

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

Thanks for the suggestion! I have some specific changes I'd like, requested inline.

Long term I'd like to restructure the docs, but that's not a blocker here.

$ _PIP_COMPILE_COMPLETE=fish_source pip-compile > ~/.config/fish/completions/pip-compile.fish
$ _PIP_SYNC_COMPLETE=fish_source pip-sync > ~/.config/fish/completions/pip-sync.fish
```

Copy link
Member

Choose a reason for hiding this comment

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

I would rather we didn't instruct users to modify their profile (~/.bashrc or ~/.zshrc), although with fish it's almost unavoidable.

I've found many times that

  • an experienced user doesn't need such instruction
  • novice users will run commands which modify their profile multiple times, resulting in a repetitive and sometimes broken profile

Can we instead provide the eval-based version of this, and trust that users who need to use the non-eval version know what to do?

In particular, I'd like us to imitate the current click docs: https://click.palletsprojects.com/en/stable/shell-completion/#enabling-completion

Note that the docs say "add this to ~/.bashrc" and don't provide a command for doing so automatically.

So I'd much rather see:

  • for bash, add these two evals to your ~/.bashrc
  • for zsh, add these two evals to your ~/.zshrc
  • leave the fish doc alone

Not only is fish more complicated to do any other way, it's also a much more niche shell, with typically more advanced users.

Copy link
Member

Choose a reason for hiding this comment

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

When I said "leave the fish doc alone", I meant that the prior version looked correct to me, namely:

$ _PIP_COMPILE_COMPLETE=fish_source pip-compile > ~/.config/fish/completions/pip-compile.fish
$ _PIP_SYNC_COMPLETE=fish_source pip-sync > ~/.config/fish/completions/pip-sync.fish

The current version is telling people to put the source snippet into a dedicated completion file, which is a very unusual way to do it and not in line with how click documents this feature.

Could you please revert that part?

- Add contributor credit to changelog
- Remove "via click" implementation detail from docs
- Use eval-based approach following Click docs style
- Keep Fish instructions separate (per reviewer preference)

Author: Ahmed Adel Bakr Alderai
@Ahmed-AdelB
Copy link
Author

@sirosen Thank you for the review! I've addressed all your feedback:

  1. ✅ Added contributor credit to changelog with {user} syntax
  2. ✅ Removed "via click" implementation detail
  3. ✅ Changed to eval-based approach following Click docs style
  4. ✅ Kept Fish instructions separate with their own files

Please let me know if any further changes are needed.


Ahmed Adel Bakr Alderai

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[advice] pip-{compile,sync} have completions, why not add some document about it to README.md?

2 participants