feat(fish): list fish abbreviations and aliases with descriptions - #219
Conversation
Adds a function that dynamically lists current fish abbreviations and aliases. It extracts descriptions from function --description flags or leading comments.
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new Fish shell function fish_shortcuts that dynamically lists all current abbreviations and aliases with their expansions and automatically inferred descriptions. The function intelligently extracts descriptions from function definitions when expansions map to single functions.
Key changes:
- Creates a comprehensive listing system for Fish shell shortcuts with automatic description extraction
- Implements smart parsing to extract function descriptions from
--descriptionflags or leading comments - Provides formatted tabular output showing shortcut type, name, expansion, and optional description
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| set -l abbr_show (abbr --show 2>/dev/null) | ||
|
|
||
| for a in $abbr_list | ||
| # Find the matching line for this abbreviation in the abbr --show output |
There was a problem hiding this comment.
[nitpick] The regex construction using string concatenation is complex and hard to read. Consider using a simpler approach or adding a comment explaining the pattern logic, especially the purpose of (\b|\s).* at the end.
| # Find the matching line for this abbreviation in the abbr --show output | |
| # Find the matching line for this abbreviation in the abbr --show output | |
| # The regex matches lines that start with the abbreviation name as a whole word, | |
| # followed by a word boundary (\b) or whitespace (\s), then any characters. |
| # If the expansion is a single token and is a function, derive description | ||
| if test -n "$expansion" | ||
| set -l first_token (string split ' ' -- $expansion)[1] | ||
| if test (count (string split ' ' -- $expansion)) -eq 1; and functions -q $first_token |
There was a problem hiding this comment.
The string split operation is performed twice - once to get the count and once to get the first token (line 86). Consider storing the split result in a variable to avoid redundant operations.
| set -l desc "" | ||
| if test -n "$expansion" | ||
| set -l first_token (string split ' ' -- $expansion)[1] | ||
| if test (count (string split ' ' -- $expansion)) -eq 1; and functions -q $first_token |
There was a problem hiding this comment.
Same issue as with abbreviations - string split is performed twice (here and on line 106). Store the split result in a variable to avoid redundant operations.
There was a problem hiding this comment.
Code Review
This pull request introduces a very useful fish_shortcuts function to list abbreviations and aliases. The implementation is well-structured, especially the logic to infer descriptions from function definitions.
My review includes a few suggestions to improve robustness and performance:
- A fix for a bug where empty function descriptions are not correctly parsed.
- A significant performance and correctness improvement for how abbreviations are processed.
- A suggestion to refactor duplicated code into a helper function for better maintainability.
- A note on the brittleness of parsing alias definitions.
Overall, this is a great addition. Addressing these points will make the script more reliable and efficient.
| # Try double-quoted first | ||
| set -l d (string replace -r -- '.*--description(=| )\"([^\"]+)\".*' '$2' -- $desc_line) | ||
| if test -n "$d" | ||
| echo $d | ||
| return 0 | ||
| end | ||
| # Fallback to single-quoted | ||
| set -l d (string replace -r -- ".*--description(=| )'([^']+)'.*" '$2' -- $desc_line) | ||
| if test -n "$d" | ||
| echo $d | ||
| return 0 | ||
| end |
There was a problem hiding this comment.
There are two issues in this block that prevent correctly parsing empty descriptions (e.g., --description "").
- The regex quantifiers
+should be*to match zero-or-more characters inside the quotes. - The check
test -n "$d"will be false for an empty description, causing the logic to fall through incorrectly. You should check ifstring replaceactually performed a replacement by comparing the result with the original string (test "$d" != "$desc_line").
# Try double-quoted first
set -l d (string replace -r -- '.*--description(=| )\"([^\"]*)\".*' '$2' -- $desc_line)
if test "$d" != "$desc_line"
echo $d
return 0
end
# Fallback to single-quoted
set -l d (string replace -r -- ".*--description(=| )'([^']*)'.*" '$2' -- $desc_line)
if test "$d" != "$desc_line"
echo $d
return 0
end
| set -l abbr_list (abbr --list 2>/dev/null) | ||
| set -l abbr_show (abbr --show 2>/dev/null) | ||
|
|
||
| for a in $abbr_list | ||
| # Find the matching line for this abbreviation in the abbr --show output | ||
| set -l line (printf "%s\n" $abbr_show | string match -r -- "^"(string escape --style=regex -- $a)"(\b|\s).*") | ||
| set -l expansion "" | ||
| if test -n "$line" | ||
| # Extract the expansion part after '->' | ||
| set expansion (string replace -r -- '^[^>]+->\s*' '' -- $line) | ||
| end | ||
|
|
||
| set -l desc "" | ||
| # If the expansion is a single token and is a function, derive description | ||
| if test -n "$expansion" | ||
| set -l first_token (string split ' ' -- $expansion)[1] | ||
| if test (count (string split ' ' -- $expansion)) -eq 1; and functions -q $first_token | ||
| set desc (__fish__get_func_desc $first_token) | ||
| end | ||
| end | ||
|
|
||
| __fish__format_row abbr $a "$expansion" "$desc" | ||
| set any_output 1 | ||
| end |
There was a problem hiding this comment.
The current implementation for processing abbreviations is inefficient and contains a bug.
- Bug:
abbr --showproduces recreation commands (e.g.,abbr -a ...), but the parsing logic expectsname -> expansionformat, which comes fromabbrwithout arguments. This will cause the script to fail to find any abbreviations. - Inefficiency: The code iterates through abbreviation names and for each name, it searches through the full list of abbreviation definitions. This is an O(n²) operation.
You can fix the bug and improve performance to O(n) by iterating directly over the output of abbr and parsing each line.
for line in (abbr 2>/dev/null)
set -l parts (string split -m 2 ' -> ' -- $line)
if test (count $parts) -ne 2
continue
end
set -l a $parts[1]
set -l expansion $parts[2]
set -l desc ""
# If the expansion is a single token and is a function, derive description
if test -n "$expansion"
set -l first_token (string split ' ' -- $expansion)[1]
if test (count (string split ' ' -- $expansion)) -eq 1; and functions -q $first_token
set desc (__fish__get_func_desc $first_token)
end
end
__fish__format_row abbr $a "$expansion" "$desc"
set any_output 1
end
| set -l desc "" | ||
| # If the expansion is a single token and is a function, derive description | ||
| if test -n "$expansion" | ||
| set -l first_token (string split ' ' -- $expansion)[1] | ||
| if test (count (string split ' ' -- $expansion)) -eq 1; and functions -q $first_token | ||
| set desc (__fish__get_func_desc $first_token) | ||
| end | ||
| end |
There was a problem hiding this comment.
This block of code, which derives a function description from an expansion, is duplicated in the alias processing section (lines 104-110). To improve maintainability and adhere to the DRY (Don't Repeat Yourself) principle, you should extract this logic into a new helper function.
For example, you could create a function like this:
function __fish__get_desc_from_expansion --argument-names expansion
if test -z "$expansion"
return 1
end
set -l tokens (string split ' ' -- $expansion)
if test (count $tokens) -eq 1; and functions -q $tokens[1]
__fish__get_func_desc $tokens[1]
end
endThen you could call it in both places: set desc (__fish__get_desc_from_expansion "$expansion").
| for line in (alias 2>/dev/null) | ||
| # line format typically: alias NAME 'EXPANSION...' | ||
| set -l name (string replace -r -- '^alias\s+([^\s]+).*$' '$1' -- $line) | ||
| set -l expansion (string replace -r -- "^alias\s+[^\s]+\s+'([^']*)'.*$" '$1' -- $line) |
There was a problem hiding this comment.
The regular expression used to extract the alias expansion is not fully robust. It assumes the expansion is wrapped in single quotes and does not contain any escaped single quotes. This will fail for aliases with more complex definitions, for example, an alias containing a single quote in its definition. While parsing alias output is known to be tricky, it's worth noting this limitation.
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on October 27
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| if test -n "$line" | ||
| # Extract the expansion part after '->' | ||
| set expansion (string replace -r -- '^[^>]+->\s*' '' -- $line) | ||
| end |
There was a problem hiding this comment.
Bug: Fish Shortcuts Misparse Abbreviation Output
The fish_shortcuts function misinterprets abbr --show output. It expects NAME -> EXPANSION and the abbreviation name at the line start, but abbr --show actually uses abbr -a -- NAME 'EXPANSION'. This leads to incorrect parsing and abbreviations displaying with empty expansions.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
WalkthroughUpdates Fish shell setup: renames the env loader function and adjusts its invocation, adds a new shortcuts abbreviation, introduces a new shortcuts introspection utility, adds descriptions to several functions, and changes Codex prompt functions to run even with empty arguments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Fish as Fish shell
participant Abbr as Abbreviation "shortcuts"
participant Fn as _fish_shortcuts
participant AbbrAPI as abbr/alias
participant FS as functions dir
User->>Fish: type "shortcuts"
Fish->>Abbr: expand
Abbr->>Fn: invoke
rect rgba(200,220,255,0.25)
note right of Fn: Collect metadata
Fn->>AbbrAPI: abbr --list / --show
Fn->>AbbrAPI: alias
Fn->>FS: scan/summarize functions
end
Fn-->>User: print formatted rows (abbr|alias|func)
sequenceDiagram
autonumber
actor User
participant Fish as Fish shell
participant CXE as _cxe_function / _clxe_function
participant Codex as codex
User->>Fish: _cxe_function [args...]
Fish->>CXE: call
alt no args
CXE->>Codex: run with default options (empty prompt)
else args provided
CXE->>Codex: run with joined prompt
end
Codex-->>User: output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
- _clxe_function: Run Codex with a free-form prompt using the local gpt-oss:120b model - _cxe_function: Run Codex with a free-form prompt - _gco_function: Checkout default branch and pull latest changes - _grco_function: Hard reset default branch to remote state - _grcr_function: Hard reset current branch to remote state
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
home-manager/programs/fish/functions/_fish_shortcuts.fish (3)
31-38: Update comment and consider truncation handling.Line 32's comment lists
"abbr" | "alias"but this function is also called with"func"on line 233. Update the comment to include all valid values.Additionally, the fixed column widths (%-20s, %-40s) may truncate long names or expansions without indication. Consider whether truncation is acceptable or if the output should adapt to content length.
- # kind: "abbr" | "alias" + # kind: "abbr" | "alias" | "func"
59-97: Python parsing could fail silently.The Python parsing block (lines 61-89) can fail for various reasons (syntax errors, import failures, etc.), but errors are not captured or logged. If the Python script fails,
$parsedwill be empty and parsing silently falls back to regex, which may be less accurate.Consider capturing stderr to log parsing failures for debugging:
set -lx __FISH_SHORTCUT_LINE "$line" - set -l parsed (python3 -c " + set -l parsed (python3 -c " import os, shlex line = os.environ['__FISH_SHORTCUT_LINE'] tokens = shlex.split(line) name = '' expansion = '' try: idx = tokens.index('--') except ValueError: pass else: if idx + 1 < len(tokens): name = tokens[idx + 1] rest = tokens[idx + 2:] if '--function' in tokens: try: pos = tokens.index('--function') except ValueError: pos = -1 if pos != -1 and pos + 1 < len(tokens): expansion = tokens[pos + 1] if not expansion and rest: expansion = ' '.join(rest) print(name) print(expansion) -") +" 2>&1) + set -l python_status $status + if test $python_status -ne 0 + # Python parsing failed, will fall back to regex + end set -e __FISH_SHORTCUT_LINE
159-165: Alias parsing assumes a specific output format.The parsing logic assumes
aliasoutput follows the formatalias NAME 'EXPANSION', but the format can vary across Fish versions or configurations. If the format differs (e.g., double quotes, no quotes, or additional flags), parsing may fail silently.Consider adding validation or error messages for unexpected formats.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
home-manager/programs/fish/default.nix(2 hunks)home-manager/programs/fish/functions/_clxe_function.fish(1 hunks)home-manager/programs/fish/functions/_cxe_function.fish(1 hunks)home-manager/programs/fish/functions/_fish_shortcuts.fish(1 hunks)home-manager/programs/fish/functions/_gco_function.fish(1 hunks)home-manager/programs/fish/functions/_grco_function.fish(1 hunks)home-manager/programs/fish/functions/_grcr_function.fish(1 hunks)home-manager/programs/fish/functions/_hm_load_env_file.fish(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
home-manager/**
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
Keep home-manager configurations under home-manager/
Files:
home-manager/programs/fish/functions/_hm_load_env_file.fishhome-manager/programs/fish/functions/_grco_function.fishhome-manager/programs/fish/functions/_gco_function.fishhome-manager/programs/fish/default.nixhome-manager/programs/fish/functions/_grcr_function.fishhome-manager/programs/fish/functions/_cxe_function.fishhome-manager/programs/fish/functions/_fish_shortcuts.fishhome-manager/programs/fish/functions/_clxe_function.fish
**/*.nix
📄 CodeRabbit inference engine (CLAUDE.md)
Format all Nix files with nixfmt
**/*.nix: Nix: Use 2 spaces for indentation
Nix: Keep line length under 100 characters
Nix: Sort attribute sets alphabetically
Nix: Use consistent spacing around operators
Nix: Format lists and sets consistentlyFollow the Nix expression language style guide
Files:
home-manager/programs/fish/default.nix
**/default.nix
📄 CodeRabbit inference engine (CLAUDE.md)
Use default.nix files for module exports
Files:
home-manager/programs/fish/default.nix
home-manager/programs/*/default.nix
📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
home-manager/programs/*/default.nix: Program configurations must be located in home-manager/programs// with configuration in default.nix
Program configurations should prefer Home Manager’s built-in modules when available
Program configurations should include all necessary dependencies
Program configurations should follow the provided template (programs..enable, package, and settings attrset)
Files:
home-manager/programs/fish/default.nix
home-manager/**/*.nix
📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
Use proper indentation and formatting in Nix files
Files:
home-manager/programs/fish/default.nix
home-manager/programs/**/default.nix
📄 CodeRabbit inference engine (.cursor/rules/nix.mdc)
Program configurations in home-manager/programs should be organized by program name (one directory per program with a default.nix)
Files:
home-manager/programs/fish/default.nix
home-manager/programs/**/*.nix
📄 CodeRabbit inference engine (.cursor/rules/nix.mdc)
home-manager/programs/**/*.nix: Program configurations should include all necessary dependencies
Use home.packages for package installations in program configurations
Use programs. options provided by Home Manager when available
Files:
home-manager/programs/fish/default.nix
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: nix-darwin
- GitHub Check: e2e-run (Ubuntu, ubuntu-latest)
- GitHub Check: nix-nixos
- GitHub Check: nix-linux
- GitHub Check: e2e-run (NixOS, ubuntu-latest)
- GitHub Check: e2e-run (MacOS, macos-latest)
- GitHub Check: docker-build-push (linux/amd64, -amd64, amd64)
🔇 Additional comments (12)
home-manager/programs/fish/functions/_grcr_function.fish (1)
1-1: LGTM! Description accurately reflects the function's behavior.The description "Hard reset current branch to remote state" is clear and concise, accurately describing the function's purpose for the fish_shortcuts utility.
home-manager/programs/fish/functions/_gco_function.fish (1)
1-1: LGTM! Description matches the function behavior.The added description accurately summarizes the function's purpose of checking out the default branch and pulling latest changes.
home-manager/programs/fish/functions/_grco_function.fish (1)
1-1: LGTM! Description accurately captures the function's purpose.The description clearly indicates this function hard resets the default branch (as opposed to the current branch in
_grcr_function), which accurately reflects the implementation.home-manager/programs/fish/functions/_cxe_function.fish (2)
5-10: Verify that running Codex with no prompt is intended behavior.The function now executes Codex even when no arguments are provided (line 6), whereas previously (based on the AI summary) it would have shown a usage message. Please confirm this behavior change is intentional, as it may invoke Codex in an interactive or default mode that could be unexpected.
1-1: LGTM! Description is clear and accurate.The description properly conveys that this function runs Codex with a free-form prompt.
home-manager/programs/fish/functions/_clxe_function.fish (2)
5-10: Verify that running Codex with no prompt is intended behavior.Similar to
_cxe_function, this function now executes Codex even when no arguments are provided. Please confirm this is the desired behavior, as it represents a change from requiring a prompt to allowing invocation without one.
1-1: LGTM! Description is specific and informative.The description clearly indicates both the general purpose (running Codex with a prompt) and the specific model/profile being used (local gpt-oss:120b), which is helpful for users.
home-manager/programs/fish/functions/_fish_shortcuts.fish (2)
210-225: Sourcing functions may have unexpected side effects.Line 224 sources arbitrary
.fishfiles from the functions directory. While2>/dev/nullsuppresses errors, successful sourcing can still execute arbitrary code or modify the shell environment. Functions with side effects in their definition (e.g., setting global variables, creating aliases) will affect the current shell.Consider whether this behavior is acceptable or if functions should be inspected without sourcing them (e.g., parsing function definitions from file content).
243-247: LGTM!The conditional wrapper pattern is well-implemented. Using
--wrapsensures completion inheritance, and checking for existing definitions avoids conflicts.home-manager/programs/fish/functions/_hm_load_env_file.fish (1)
1-1: LGTM! Function rename aligns with conventions.The rename from
__hm_load_env_fileto_hm_load_env_file(double to single underscore) is consistent with other internal function names in this PR and follows Fish conventions where single underscore indicates an internal/helper function.home-manager/programs/fish/default.nix (2)
16-16: LGTM! Function call updated correctly.The call to
_hm_load_env_filecorrectly reflects the function rename in_hm_load_env_file.fish.
59-59: LGTM! New abbreviation exposes shortcuts utility.The new
shortcutsabbreviation provides convenient access to the_fish_shortcutsutility introduced in this PR. This aligns with the PR objectives to make abbreviations and aliases discoverable.
Adds fish_shortcuts function to list abbreviations and aliases dynamically.
What’s included
Usage
Notes
Co-authored-by: Agent Mode ai-agent@users.noreply.github.com