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 $N placeholder now keeps the arguments after it. Before, it dropped them without a warning: a glab api graphql alias whose query declares a variable such as $fullPath never 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 $1 could 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, so mr list $1 called with "" expanded to mr list.
  • $0 is literal text. Before, any alias containing $0 failed with not enough arguments for alias.
  • A $ followed by several digits is one placeholder, so $11 is the eleventh argument. Before, it was the first argument followed by 1, 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.

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 error

The 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.015s

All 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.

Edited by José M. Requena Plens

Merge request reports

Loading
Loading