Skip to content

fix(completions): ignore aliases and functions named usage (2nd attempt) #304

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

risu729
Copy link
Contributor

@risu729 risu729 commented Jul 16, 2025

Reverts #301.

Replaces command -v usage with type -P usage to ignore aliases and functions.
In the 1st attempt, I used command usage, but since usage (or usage --help) exits with an exit code 2, it didn't work as an existence check.

I apologise for the trouble. Thank you for your help as always.

@Copilot Copilot AI review requested due to automatic review settings July 16, 2025 19:30
Copy link

codecov bot commented Jul 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.23%. Comparing base (d5cdaf5) to head (dfdc67b).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #304   +/-   ##
=======================================
  Coverage   63.23%   63.23%           
=======================================
  Files          42       42           
  Lines        3454     3454           
  Branches     3454     3454           
=======================================
  Hits         2184     2184           
  Misses        982      982           
  Partials      288      288           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

This comment was marked as outdated.

@jdx
Copy link
Owner

jdx commented Jul 17, 2025

bugbot run

@@ -15,7 +15,7 @@ _usage() {
typeset -A opt_args
local curcontext="$curcontext" spec cache_policy

if ! command -v usage &> /dev/null; then
if ! type -P usage &> /dev/null; then
Copy link
Owner

Choose a reason for hiding this comment

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

it looks like you need to redirect stdout not stderr for type -P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

&> will redirect both stdout and stderr, so it should be fine.

@risu729 risu729 marked this pull request as draft July 17, 2025 14:11
@risu729
Copy link
Contributor Author

risu729 commented Jul 18, 2025

I confirmed type -p works fine for both bash and zsh.

bash

$ type -p usage
/home/risu/.local/share/mise/installs/usage/2.2.2/usage
$ type -p beep

zsh

% type -p usage
usage is /home/risu/.local/share/mise/installs/usage/2.2.2/usage
% type -p beep
beep not found
% which beep
beep: aliased to printf '\a'

@risu729 risu729 requested a review from Copilot July 18, 2025 19:53
@risu729 risu729 marked this pull request as ready for review July 18, 2025 19:53
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes shell completion scripts to properly ignore aliases and functions named "usage" when checking for the usage CLI tool's existence. The change replaces command -v with type -p for existence checks and prefixes direct usage calls with command to bypass aliases/functions.

Key changes:

  • Replace command -v usage with type -p usage for CLI existence checks
  • Prefix direct usage command calls with command to bypass aliases/functions
  • Update all affected shell completion files and test snapshots

Reviewed Changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/src/complete/zsh.rs Updates zsh completion template to use type -p for checks and command usage for calls
lib/src/complete/fish.rs Updates fish completion template with same command resolution fixes
lib/src/complete/bash.rs Updates bash completion template with same command resolution fixes
lib/bash-completion/bash_completion Changes type -P to type -p for consistency
cli/assets/completions/* Updates static completion files for all shells
lib/src/complete/snapshots/* Updates test snapshots to reflect the template changes

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