-
Notifications
You must be signed in to change notification settings - Fork 5k
feat(shell): add fish support to create-new-feature
#1361
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,297 @@ | ||||||||||||||
| #!/usr/bin/env fish | ||||||||||||||
|
|
||||||||||||||
| set JSON_MODE false | ||||||||||||||
| set SHORT_NAME "" | ||||||||||||||
| set BRANCH_NUMBER "" | ||||||||||||||
| set ARGS | ||||||||||||||
|
|
||||||||||||||
| set i 1 | ||||||||||||||
| while test $i -le (count $argv) | ||||||||||||||
| set arg $argv[$i] | ||||||||||||||
| switch $arg | ||||||||||||||
| case --json | ||||||||||||||
| set JSON_MODE true | ||||||||||||||
| case --short-name | ||||||||||||||
| if test (math $i + 1) -gt (count $argv) | ||||||||||||||
| echo 'Error: --short-name requires a value' >&2 | ||||||||||||||
| exit 1 | ||||||||||||||
| end | ||||||||||||||
| set i (math $i + 1) | ||||||||||||||
| set next_arg $argv[$i] | ||||||||||||||
| # Check if the next argument is another option | ||||||||||||||
| if string match -q -- '--*' $next_arg | ||||||||||||||
| echo 'Error: --short-name requires a value' >&2 | ||||||||||||||
| exit 1 | ||||||||||||||
| end | ||||||||||||||
| set SHORT_NAME $next_arg | ||||||||||||||
| case --number | ||||||||||||||
| if test (math $i + 1) -gt (count $argv) | ||||||||||||||
| echo 'Error: --number requires a value' >&2 | ||||||||||||||
| exit 1 | ||||||||||||||
| end | ||||||||||||||
| set i (math $i + 1) | ||||||||||||||
| set next_arg $argv[$i] | ||||||||||||||
| if string match -q -- '--*' $next_arg | ||||||||||||||
| echo 'Error: --number requires a value' >&2 | ||||||||||||||
| exit 1 | ||||||||||||||
| end | ||||||||||||||
| set BRANCH_NUMBER $next_arg | ||||||||||||||
| case --help -h | ||||||||||||||
| echo "Usage: create-new-feature.fish [--json] [--short-name <name>] [--number N] <feature_description>" | ||||||||||||||
| echo "" | ||||||||||||||
| echo "Options:" | ||||||||||||||
| echo " --json Output in JSON format" | ||||||||||||||
| echo " --short-name <name> Provide a custom short name (2-4 words) for the branch" | ||||||||||||||
| echo " --number N Specify branch number manually (overrides auto-detection)" | ||||||||||||||
| echo " --help, -h Show this help message" | ||||||||||||||
| echo "" | ||||||||||||||
| echo "Examples:" | ||||||||||||||
| echo " create-new-feature.fish 'Add user authentication system' --short-name 'user-auth'" | ||||||||||||||
| echo " create-new-feature.fish 'Implement OAuth2 integration for API' --number 5" | ||||||||||||||
|
Comment on lines
+40
to
+50
|
||||||||||||||
| exit 0 | ||||||||||||||
| case '*' | ||||||||||||||
| set -a ARGS $arg | ||||||||||||||
| end | ||||||||||||||
| set i (math $i + 1) | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| set FEATURE_DESCRIPTION (string join " " $ARGS) | ||||||||||||||
| if test -z "$FEATURE_DESCRIPTION" | ||||||||||||||
| echo "Usage: create-new-feature.fish [--json] [--short-name <name>] [--number N] <feature_description>" >&2 | ||||||||||||||
|
||||||||||||||
| exit 1 | ||||||||||||||
| end | ||||||||||||||
|
|
||||||||||||||
| # Function to find the repository root | ||||||||||||||
|
||||||||||||||
| # Function to find the repository root | |
| # Function to find the repository root by searching for existing project markers |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing base-10 interpretation prefix. The bash version uses number=$((10#$number)) to force base-10 interpretation. This fish version only uses (math $number + 0) which may not properly handle numbers with leading zeros (e.g., "008" could be misinterpreted). This could lead to incorrect identification of the highest spec number when directory names have leading zeros.
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing base-10 interpretation prefix. The bash version uses number=$((10#$number)) to force base-10 interpretation. This fish version only uses (math $number + 0) which may not properly handle numbers with leading zeros (e.g., "008" could be misinterpreted). This could lead to incorrect identification of the highest branch number when branch names have leading zeros.
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing detailed comment. The bash version has "Function to check existing branches (local and remote) and return next available number" which is more descriptive. The fish version only has "Function to check existing branches" without mentioning "local and remote" or "return next available number". Consider adding the full description for better clarity.
| # Function to check existing branches | |
| # Function to check existing branches (local and remote) and return next available number |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing comment about remote fetching. The bash version has "Fetch all remotes to get latest branch info (suppress errors if no remotes)" which explains both the action and the error suppression strategy. The fish version lacks this comment. Consider adding it for better documentation.
| # Fetch all remotes | |
| # Fetch all remotes to get latest branch info (suppress errors if no remotes) |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing detailed comment about repository root detection. The bash version includes an explanatory comment: "Resolve repository root. Prefer git information when available, but fall back to searching for repository markers so the workflow still functions in repositories that were initialised with --no-git." This provides important context about the design decision. Consider adding this comment for better maintainability.
| # Resolve repository root | |
| # Resolve repository root. Prefer git information when available, but fall back | |
| # to searching for repository markers so the workflow still functions in | |
| # repositories that were initialised with --no-git. |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing detailed comment explaining generation logic. The bash version has: "Function to generate branch name with stop word filtering and length filtering" while fish version only says "Function to generate branch name with stop word filtering". The "and length filtering" part describes an important aspect of the function's behavior. Consider adding this for accuracy and consistency.
| # Function to generate branch name with stop word filtering | |
| # Function to generate branch name with stop word and length filtering |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stop words regex pattern is missing the word boundary anchors that are present in the bash version. The bash version uses grep -qiE "$stop_words" which effectively matches whole words. Without proper word boundary handling in fish, partial matches could occur. Consider revising the regex pattern to ensure only complete word matches are filtered.
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment differs from bash version. The bash version says "Filter words: remove stop words and words shorter than 3 chars (unless they're uppercase acronyms in original)" which is more descriptive. The fish version says "Filter words" followed by "Keep words that are NOT stop words AND (length >= 3 OR are potential acronyms)" in a later comment. Consider matching the bash version's comment style for consistency.
| # Filter words | |
| # Filter words: remove stop words and words shorter than 3 chars (unless they're uppercase acronyms in original) |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The acronym detection logic differs from the bash version. The bash version checks if the word appears as uppercase in the original description using grep -q "\b${word^^}\b" with word boundaries. The fish version uses string match -q "*"(string upper $word)"*" which performs a substring match and could produce false positives. For example, if the word "id" appears anywhere in "valid", it would match. Consider using a more precise matching approach.
| else if echo $description | string match -q "*"(string upper $word)"*" | |
| # Keep short words if they appear as uppercase in original | |
| else if echo $description | grep -q -w (string upper $word) | |
| # Keep short words if they appear as uppercase in original as a whole word |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback branch name generation logic differs from the bash version. The bash version uses tr '\n' '-' | sed 's/-$//' to join words and remove trailing hyphens, while the fish version uses string join '-'. While both achieve similar results, the bash version explicitly processes the cleaned output through tr '-' '\n' | grep -v '^$' | head -3 to split, filter empty strings, take first 3, then rejoin. The fish version uses string split '-' | string match -v '' | head -3 | string join '-' which should work equivalently. However, verify this produces identical results in edge cases.
| # Fallback to original logic | |
| set cleaned (clean_branch_name $description) | |
| echo $cleaned | string split '-' | string match -v '' | head -3 | string join '-' | |
| # Fallback to original logic (match bash implementation) | |
| set cleaned (clean_branch_name $description) | |
| echo $cleaned | tr '-' '\n' | grep -v '^$' | head -3 | tr '\n' '-' | sed 's/-$//' |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent comment format. The bash version has a more detailed comment explaining "Force base-10 interpretation to prevent octal conversion (e.g., 010 → 8 in octal, but should be 10 in decimal)". This fish version only says "Force base-10 interpretation" without the helpful explanation. For maintainability and clarity, consider adding the full explanation as in the bash version.
| # Force base-10 interpretation | |
| # Force base-10 interpretation to prevent octal conversion (e.g., 010 → 8 in octal, but should be 10 in decimal) |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing base-10 interpretation prefix. The bash version uses $((10#$BRANCH_NUMBER)) to force base-10 interpretation and prevent octal conversion (e.g., "010" being interpreted as octal 8 instead of decimal 10). The fish version only uses (math $BRANCH_NUMBER + 0) which may not handle this edge case correctly. This could lead to incorrect branch numbering when branch numbers like 008, 009, 010, etc. are provided.
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing comment for branch name validation section. The bash version includes a comment "Validate and truncate if necessary" after the main comment about GitHub's 244-byte limit. This helps clarify the purpose of the following code block. Consider adding this comment for consistency and clarity.
| # GitHub enforces a 244-byte limit on branch names | |
| # GitHub enforces a 244-byte limit on branch names | |
| # Validate and truncate if necessary |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment style differs from bash version. The bash version says "Calculate how much we need to trim from suffix" followed by "Account for: feature number (3) + hyphen (1) = 4 chars" which provides helpful arithmetic. The fish version only has the first comment. Consider adding the detailed breakdown for better clarity.
| # Calculate how much we need to trim from suffix | |
| # Calculate how much we need to trim from suffix | |
| # Account for: feature number (3) + hyphen (1) = 4 chars |
Copilot
AI
Dec 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing comment about truncation strategy. The bash version has "Truncate suffix at word boundary if possible" before the truncation line, explaining the intent. This comment is missing in the fish version. Consider adding it for better code documentation.
| # Truncate suffix | |
| # Truncate suffix at word boundary if possible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The script does not set error handling mode. The bash version uses
set -eto exit on errors. In fish, consider addingset -eat the beginning to ensure the script exits on command failures, providing consistent behavior with the bash implementation.