Use prefixFor so flag suggestions agree with help on rune count - #2448
Conversation
Replace the two new tests and the flagStringForTest helper with a case in TestSuggestFlag. prefixFor with a single-rune Unicode name is tested by TestCommand_SingleRuneUnicodeFlag, and the suggestFlagFromError path is covered by TestSuggestFlagFromError. Signed-off-by: Eng Zer Jun <engzerjun@gmail.com>
Juneezee
left a comment
There was a problem hiding this comment.
Thanks for the fix. Reusing prefixFor is the right call, and it makes the suggestion match the help output. I pushed a commit that trims the tests to one case in TestSuggestFlag: bc3e336
I noticed that you appear to be using an AI agent to open dozens of issues and PRs across many projects over the past week. At least one project has already rejected one of them under its AI policy: pallets/click#3879
urfave/cli has no rule against AI-assisted contributions, but every PR takes maintainer time, and this one needed a follow-up commit before it could be merged. Before opening more PRs here, please make sure you have read and understood the change yourself, keep the tests to what the change needs, and keep the description short and focused on the change. PRs that look unreviewed may be closed without review.
"Did you mean" suggests
--éfor a flag that help renders as-éDescription
suggestions.go, insuggestFlag:len()counts bytes, but a single-rune flag name can be multi-byte. Everywhere else in the library the same "one dash vs two dashes" decision is made by rune count —prefixForindocs.go:11-19:Why it's wrong
Help output and the "Did you mean …?" hint disagree about the same flag. A
BoolFlag{Name: "é"}is advertised in help as-é(viaprefixFor) but was suggested as--é, so the user is told to type a spelling the help text never showed them.Reproduction (real output, before the fix)
Note the first assertion in that subtest passes: help really does render
-é. Only the suggestion says--é.End-to-end on unmodified HEAD:
After the fix the same invocation prints
Did you mean "-é"?.The fix
Reusing
prefixForis the point: the correct helper already exists in the same package, so this removes the duplicated (and wrong) decision rather than adding a third one.Tests
TestSuggestFlagMultibyteRunePrefix(table-driven, 2 subtests) asserts the suggestion agrees withstringifyFlag's own prefix.TestSuggestFlagMultibyteRuneFromErrorasserts the exactDid you mean "-é"?string from the end-to-end path.go build ./...andgo vet ./...clean. No public API change —suggestFlagis unexported — so thev3diffgodoc gate needs no regeneration.Upstream status
Nothing open covers this. I deliberately avoided the areas already claimed: #2443 (non-last value flag in a short group), #2396 (duplicate flag names /
helpclash), #2422 (inherited persistent flags in help), #2382, #2261, #2376/#2384, #2209/#2212, #2342/#2379. Issue #2320 is a question about slice-flag help duplication, unrelated.Other candidates I left alone
FlagBase.countis never reset byPreParse, so re-running the same*CommandmakesOnlyOncefail andcmd.Count()double-count on the second invocation (flag_impl.go:79,185).errRequiredFlags.Error()wraps all names in one quote pair, so it rendersRequired flags "a, b" not setinstead of"a", "b"(errors.go:60-61).SuggestFlagincludes hidden flags in suggestions, which contradicts their purpose (suggestions.go:109).