Skip to content

fix: handle a multi-byte flag at the end of a short option group - #2438

Open
SulimanAbdulrazzaq wants to merge 1 commit into
urfave:mainfrom
SulimanAbdulrazzaq:fix/short-option-multibyte-last-flag
Open

SulimanAbdulrazzaq wants to merge 1 commit into
urfave:mainfrom
SulimanAbdulrazzaq:fix/short-option-multibyte-last-flag

Conversation

@SulimanAbdulrazzaq

Copy link
Copy Markdown

What type of PR is this?

  • bug

What this PR does / why we need it:

With UseShortOptionHandling, parseFlags splits a group such as -vש into single flags. It ranges over the group by byte offset, but it found the last flag, the only one that can take its value from the next argument, with index == len(flagName)-1. When the group ends in a multi-byte flag such as ש (the name used in #2433), that check never matches:

  • -vש value operand leaves ש unset, raises no error, and passes value on as a positional argument.
  • -vש with nothing after it does not report flag needs an argument.

Output of a small program with UseShortOptionHandling and three flags: v (bool), o (string) and ש (string):

before: ["-vש" "value" "operand"] -> v=true o="" ש="" args=["value" "operand"]
after:  ["-vש" "value" "operand"] -> v=true o="" ש="value" args=["operand"]

-vo out.txt operand and -v -ש value operand already worked before this change.

  • command_parse.go: find where the last rune of the group starts, and compare the byte offset against that in both places that used len(flagName)-1. Groups that end in an ASCII flag behave exactly as before.
  • command_test.go: TestCommand_SingleRuneUnicodeFlagEndsShortOptionGroup checks the value, the remaining arguments, and the missing-value error.

Which issue(s) this PR fixes:

There is no separate issue. This follows up #2433 / #2434, which fixed standalone single-rune Unicode flags.

Special notes for your reviewer:

Two places still choose the -/-- prefix by byte length, so they show a single-rune non-ASCII name with --. Other flags have shown since #2434.

  • BoolWithInverseFlag.String (len(bif.Name) == 1 and len(alias) == 1) shows --[no-]ש.
  • The "Did you mean" suggestion in suggestions.go (len(suggestion) == 1) suggests --ש.

Both only affect output (the parser accepts either prefix), so they are left out of this PR. They can be added here or sent separately if you prefer.

Testing

  • The new test fails on main: the value is "", the arguments are ["value" "operand"], and no error is returned when the value is missing. It passes with this change.
  • On Windows with Go 1.27.1, all of these pass:
    • go test ./...
    • go vet ./...
    • golangci-lint v2.13.2 (0 issues)
    • go run scripts/build.go with check-binary-size, generate, diffcheck and v3diff
  • The tests were run without -race.

Release Notes

Fix short option groups that end in a single-rune non-ASCII flag, such as `-vש value`, leaving that flag unset.

parseFlags ranges over a short option group by byte offset, but found
the last flag, the only one that can take its value from the next
argument, by comparing that offset with len(flagName)-1. When the group
ends in a multi-byte flag that takes a value, such as a string flag
named "ש", the check never matched, so "-vש value" left the flag unset,
raised no error and passed "value" on as a positional argument.

Compare the offset with where the last rune starts instead. Groups that
end in an ASCII flag behave exactly as before.

Follow-up to urfave#2434.
@SulimanAbdulrazzaq
SulimanAbdulrazzaq requested a review from a team as a code owner September 22, 2026 10:48

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant