fix(alias): pass alias arguments through unchanged
Description
For an alias that is not a shell alias, ExpandAlias pasted each argument into the alias text with strings.ReplaceAll and then split the result with shlex, so every argument was parsed a second time. Quotes and backslashes inside an argument were removed, an argument with an unmatched quote could make the alias fail, an argument in an unquoted placeholder was split on its spaces, and an argument starting with # in an unquoted placeholder was read as a comment and dropped together with the rest of the alias text. $N text inside an argument was taken as a placeholder: it was filled with another argument, or it failed the alias with not enough arguments for alias. While any $ was left in the text, the arguments after the placeholders were dropped without a warning, and $10 was filled with the first argument followed by 0.
This MR splits the alias text first and fills the placeholders inside each resulting word, so every argument reaches the command unchanged and as one argument. The arguments after the highest placeholder are still appended, and not enough arguments for alias keeps its message, with the arguments that were given filled in as before. Shell aliases (--shell, an expansion starting with !) are not touched.
Behavior changes, all intended:
- An alias that contains a
$but no$Nplaceholder now keeps the arguments after it. Before, it dropped them without a warning: aglab api graphqlalias whose query declares a variable such as$fullPathnever received the-f fullPath=...that sets it. - An argument filled into an unquoted placeholder (
--search=$1) stays one argument. Before, it was split on its spaces, so$1could carry several arguments. - An empty argument filled into an unquoted placeholder that is a whole word (
mr list $1) is passed on as an empty argument, as a quoted placeholder already did. Before, it was dropped, somr list $1called with""expanded tomr list. $0is literal text. Before, any alias containing$0failed withnot enough arguments for alias.- A
$followed by several digits is one placeholder, so$11is the eleventh argument. Before, it was the first argument followed by1, and a missing eleventh argument was never reported.
The second and third are the changes someone could notice: an alias such as mr list $1 called with '--draft --label x' used to pass two flags and now passes one argument, and called with "" it used to pass nothing and now passes an empty argument. Both come from the second split. The shell has already split the arguments once, and the example in the glab alias set help quotes its placeholder (--title "$1"), so I treated the second split as part of the defect. Tell me if you would rather keep it.
Related Issues
Closes #8620 (closed)
How has this been tested?
I added eleven rows to the existing TestExpandAlias table: an argument containing $ keeps the extra arguments, $5 and $2 text inside an argument stays literal, quotes inside an argument are kept, an argument with spaces in an unquoted placeholder stays one argument, an argument starting with # in an unquoted placeholder is not read as a comment, placeholders out of order still append the extra arguments, an alias with a $ but no placeholder keeps the extra arguments, $0 stays literal, $10 gets the tenth argument, and a missing tenth argument reports not enough arguments for alias. Ten of them fail on main at 1d1012f3 and all twenty-one rows pass with the change. The out-of-order row passes on both: it keeps the new code from appending after the last placeholder found instead of the highest.
With Go 1.27.2 and golangci-lint 2.14.0, on main at 1d1012f3 with this change applied, go test -race ./... passes, golangci-lint run reports 0 issues and golangci-lint fmt --diff is empty. On the branch, go run ./scripts/comment-overlap -threshold 1.0 -max-findings 0 ./cmd/... ./internal/... reports 0 findings, make check-embed check-args is clean, and the commit message and this title pass commitlint with scripts/commit-lint/.commitlintrc.js.
End to end, I built glab from main and from main with this change and ran the same aliases in an empty configuration directory, with no token and telemetry off. cs is config set editor "$1", cu is config set editor $1, createissue is the glab alias set help example, issue create --title "$1", iv is issue view $1 --comments, and pname is api graphql -f query='query($fullPath: ID!) { project(fullPath: $fullPath) { name } }':
| Invocation | main |
With this change |
|---|---|---|
glab cs 'my "ed" here' --global |
stores my ed here |
stores my "ed" here |
glab cs 'an "odd quote' --global |
EOF found when expecting closing quote, exit status 2 |
stores an "odd quote |
glab cs 'C:\tools\vim.exe' --global |
stores C:toolsvim.exe |
stores C:\tools\vim.exe |
glab cs 'costs $5 now' --global |
not enough arguments for alias, exit status 2 |
stores costs $5 now |
glab cs '$2' --global |
$2 is filled with --global, then Accepts 2 arg(s), received 1., exit status 1 |
stores $2 |
glab cu 'my editor' --global |
Accepts 2 arg(s), received 3., exit status 1 |
stores my editor |
glab createissue 'Fix "login" page' |
expands to the title Fix login page |
expands to the title Fix "login" page |
glab createissue 'Price in $' --description x |
--description x is dropped |
--description x is passed |
glab iv '#123' |
expands to issue view |
expands to issue view #123 --comments |
glab pname -f fullPath=gitlab-org/cli |
-f fullPath=gitlab-org/cli is dropped |
-f fullPath=gitlab-org/cli is passed |
The createissue, iv and pname rows were read from the GLAB_DEBUG=true output, which prints the arguments before and after the expansion.
Test output on main (without the change to alias_expand.go)
--- FAIL: TestExpandAlias (0.00s)
--- FAIL: TestExpandAlias/dollar_in_argument_keeps_the_extra_arguments (0.00s)
--- FAIL: TestExpandAlias/placeholder_text_in_argument_is_not_a_placeholder (0.00s)
--- FAIL: TestExpandAlias/placeholder_text_in_argument_is_not_filled_by_another_argument (0.00s)
--- FAIL: TestExpandAlias/quotes_in_argument_are_kept (0.00s)
--- FAIL: TestExpandAlias/argument_with_spaces_in_an_unquoted_placeholder_stays_one_argument (0.00s)
--- FAIL: TestExpandAlias/argument_starting_with_a_hash_in_an_unquoted_placeholder_is_not_a_comment (0.00s)
--- PASS: TestExpandAlias/placeholders_out_of_order_keep_the_extra_arguments (0.00s)
--- FAIL: TestExpandAlias/dollar_without_placeholder_keeps_the_extra_arguments (0.00s)
--- FAIL: TestExpandAlias/dollar_zero_is_not_a_placeholder (0.00s)
--- FAIL: TestExpandAlias/two-digit_placeholder (0.00s)
--- FAIL: TestExpandAlias/not_enough_arguments_for_a_two-digit_placeholder (0.00s)The failure messages, in the same order:
ExpandAlias() gotExpanded = [issue list --author=$coolmoney$ --assignee=$coolmoney$], want [issue list --author=$coolmoney$ --assignee=$coolmoney$ -R monalisa/testing]
got error: not enough arguments for alias: issue create --title "Costs $5 now"
ExpandAlias() gotExpanded = [issue list --author=bug --label=bug], want [issue list --author=$2 --label=bug]
ExpandAlias() gotExpanded = [issue create --title Fix login page], want [issue create --title Fix "login" page]
ExpandAlias() gotExpanded = [issue list --search=login page -R monalisa/testing], want [issue list --search=login page -R monalisa/testing]
ExpandAlias() gotExpanded = [issue view], want [issue view #123 --comments]
ExpandAlias() gotExpanded = [issue list --search=price$], want [issue list --search=price$ -R monalisa/testing]
got error: not enough arguments for alias: issue list --search="$0"
ExpandAlias() gotExpanded = [issue list --label=one --search=one0 two three four five six seven eight nine ten], want [issue list --label=one --search=ten]
expected errorThe spaces row prints the same text on both sides because the existing assertion formats with %v; the slices still differ, which is why the row fails. The ten rows that were already in the table pass.
Command: go test -count=1 -run TestExpandAlias -v ./internal/commands/alias/expand/
Test output with the change
--- PASS: TestExpandAlias (0.00s)
PASS
ok gitlab.com/gitlab-org/cli/internal/commands/alias/expand 0.015sAll twenty-one subtests of TestExpandAlias pass.
Environment
Linux x86_64, bash, Go 1.27.2 (from go.mod), golangci-lint 2.14.0 (from .tool-versions), Node.js 24 for the commit linter.