feat(shell): add fish support to common
#1360
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Fish shell support by implementing a common.fish script that provides equivalent functionality to the existing scripts/bash/common.sh. The implementation includes utilities for repository management, branch detection, feature directory lookup, and file system checks.
Key Changes:
- Added Fish shell implementation of common utility functions for the specify workflow
- Implemented fallback logic for non-git repositories
- Added support for numeric prefix-based feature directory lookup to allow multiple branches per spec
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Exactly one match - perfect! | ||
| echo "$specs_dir/$matches[1]" | ||
| else | ||
| # Multiple matches - this shouldn't happen |
There was a problem hiding this comment.
The comment says "Multiple matches - this shouldn't happen" but the bash version is more complete: "Multiple matches - this shouldn't happen with proper naming convention". Including the reason (proper naming convention) provides better context for why this scenario is unexpected.
| # Multiple matches - this shouldn't happen | |
| # Multiple matches - this shouldn't happen with proper naming convention |
| # Exactly one match - perfect! | ||
| echo "$specs_dir/$matches[1]" | ||
| else | ||
| # Multiple matches - this shouldn't happen |
There was a problem hiding this comment.
The comment is missing context. The bash version says "Return something to avoid breaking the script" which explains why a fallback value is returned in this error case. This helps readers understand the defensive programming approach.
| # Multiple matches - this shouldn't happen | |
| # Multiple matches - this shouldn't happen, but return a fallback path | |
| # so that callers still receive a usable directory and scripts don't break. |
| else | ||
| # Fall back to script location for non-git repos | ||
| set script_dir (dirname (status --current-filename)) | ||
| cd $script_dir/../../.. && pwd |
There was a problem hiding this comment.
The cd command in the fallback path permanently changes the current directory, which differs from the bash implementation that uses a subshell. This will cause the working directory to remain changed after get_repo_root returns, potentially affecting subsequent operations. Consider using a subshell or command substitution to avoid side effects.
| cd $script_dir/../../.. && pwd | |
| set -l repo_root (cd $script_dir/../../..; and pwd) | |
| if test -n "$repo_root" | |
| echo $repo_root | |
| end |
| set branch_name $argv[2] | ||
| set specs_dir "$repo_root/specs" | ||
|
|
||
| # Extract numeric prefix from branch |
There was a problem hiding this comment.
The comment says "Extract numeric prefix from branch" but is missing the more detailed explanation that the bash version has: "Extract numeric prefix from branch (e.g., '004' from '004-whatever')". The bash version provides a helpful example that clarifies the expected format.
| # Extract numeric prefix from branch | |
| # Extract numeric prefix from branch (e.g., '004' from '004-whatever') |
| echo "$argv[1]/specs/$argv[2]" | ||
| end | ||
|
|
||
| # Find feature directory by numeric prefix instead of exact branch match |
There was a problem hiding this comment.
The comment is incomplete. The bash version has a more detailed comment: "Find feature directory by numeric prefix instead of exact branch match. This allows multiple branches to work on the same spec (e.g., 004-fix-bug, 004-add-feature)". The additional context about multiple branches working on the same spec is valuable for understanding the function's purpose.
| # Find feature directory by numeric prefix instead of exact branch match | |
| # Find feature directory by numeric prefix instead of exact branch match. This allows | |
| # multiple branches to work on the same spec (e.g., 004-fix-bug, 004-add-feature). |
|
|
||
| # Handle results | ||
| if test (count $matches) -eq 0 | ||
| # No match found |
There was a problem hiding this comment.
The comment says "No match found" but the bash version includes more context: "No match found - return the branch name path (will fail later with clear error)". This additional context explains the expected behavior when no match is found, which helps with understanding the error handling strategy.
| # No match found | |
| # No match found - return the branch name path (will fail later with clear error) |
1 participant
No description provided.