forked from earthly/earthly
-
Notifications
You must be signed in to change notification settings - Fork 12
feat: add 'Did you mean?' suggestions for unknown flags #234
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
Draft
gilescope
wants to merge
5
commits into
main
Choose a base branch
from
claude/fix-issue-205-Sr8BE
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+241
−1
Draft
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,18 +2,115 @@ package flagutil | |
|
|
||
| import ( | ||
| "context" | ||
| "math" | ||
| "os" | ||
| "reflect" | ||
| "regexp" | ||
| "strings" | ||
|
|
||
| "github.com/EarthBuild/earthbuild/ast/commandflag" | ||
| "github.com/EarthBuild/earthbuild/ast/spec" | ||
| "github.com/EarthBuild/earthbuild/util/hint" | ||
| "github.com/EarthBuild/earthbuild/util/stringutil" | ||
| "github.com/agext/levenshtein" | ||
| "github.com/pkg/errors" | ||
|
|
||
| "github.com/jessevdk/go-flags" | ||
| "github.com/urfave/cli/v2" | ||
| ) | ||
|
|
||
| // extractFlagNames extracts all long flag names from a struct using reflection. | ||
| func extractFlagNames(data any) []string { | ||
| if data == nil { | ||
| return nil | ||
| } | ||
|
|
||
| v := reflect.ValueOf(data) | ||
| if v.Kind() == reflect.Ptr { | ||
| v = v.Elem() | ||
| } | ||
| if v.Kind() != reflect.Struct { | ||
| return nil | ||
| } | ||
|
|
||
| t := v.Type() | ||
| var flagNames []string | ||
| for i := range t.NumField() { | ||
| if longTag := t.Field(i).Tag.Get("long"); longTag != "" { | ||
| flagNames = append(flagNames, longTag) | ||
| } | ||
| } | ||
| return flagNames | ||
| } | ||
|
|
||
| // findClosestFlag finds the most similar flag name to the given unknown flag. | ||
| // Returns the suggested flag and whether a good suggestion was found. | ||
| func findClosestFlag(unknownFlag string, validFlags []string) (string, bool) { | ||
| if len(validFlags) == 0 { | ||
| return "", false | ||
| } | ||
|
|
||
| // Remove leading dashes from the unknown flag for comparison | ||
| unknownFlag = strings.TrimLeft(unknownFlag, "-") | ||
|
|
||
| bestMatch := "" | ||
| bestDistance := math.MaxInt | ||
|
|
||
| for _, validFlag := range validFlags { | ||
| if distance := levenshtein.Distance(unknownFlag, validFlag, nil); distance < bestDistance { | ||
| bestDistance = distance | ||
| bestMatch = validFlag | ||
| } | ||
| } | ||
|
|
||
| // Only suggest if the distance is reasonable (less than half the length of the unknown flag). | ||
| // This prevents suggesting completely unrelated flags. | ||
| // Allow at least 2 character difference for short flags. | ||
| maxDistance := max(len(unknownFlag)/2, 2) | ||
| if bestDistance <= maxDistance { | ||
| return bestMatch, true | ||
| } | ||
| return "", false | ||
| } | ||
|
|
||
| // suggestFlagIfUnknown checks if the error is about an unknown flag and adds a suggestion if possible. | ||
| func suggestFlagIfUnknown(err error, data any) error { | ||
| if err == nil { | ||
| return nil | ||
| } | ||
|
|
||
| unknownFlag, ok := extractUnknownFlagFromError(err) | ||
| if !ok { | ||
| return err | ||
| } | ||
|
|
||
| suggestion, found := findClosestFlag(unknownFlag, extractFlagNames(data)) | ||
| if !found { | ||
| return err | ||
| } | ||
|
|
||
| return hint.Wrapf(err, "Did you mean '--%s'?", suggestion) | ||
| } | ||
|
|
||
| // unknownFlagRegexp matches the flag name in go-flags error messages like "unknown flag `flag-name'". | ||
| var unknownFlagRegexp = regexp.MustCompile("`([^']+)'") | ||
|
|
||
| // extractUnknownFlagFromError extracts the flag name from an "unknown flag" error. | ||
| // Uses type assertion to check for the specific error type from go-flags library. | ||
| func extractUnknownFlagFromError(err error) (string, bool) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here, bool is redundant |
||
| var flagErr *flags.Error | ||
| if !errors.As(err, &flagErr) || flagErr.Type != flags.ErrUnknownFlag { | ||
| return "", false | ||
| } | ||
|
|
||
| matches := unknownFlagRegexp.FindStringSubmatch(flagErr.Message) | ||
| if len(matches) < 2 { | ||
| return "", false | ||
| } | ||
|
|
||
| return matches[1], true | ||
| } | ||
|
|
||
| // ArgumentModFunc accepts a flagName which corresponds to the long flag name, and a pointer | ||
| // to a flag value. The pointer is nil if no flag was given. | ||
| // the function returns a new pointer set to nil if one wants to pretend as if no value was given, | ||
|
|
@@ -81,6 +178,8 @@ func ParseArgsWithValueModifierAndOptions( | |
| if parserOptions&flags.PrintErrors != flags.None { | ||
| p.WriteHelp(os.Stderr) | ||
| } | ||
| // Try to provide helpful suggestions for unknown flags | ||
| err = suggestFlagIfUnknown(err, data) | ||
| return nil, err | ||
| } | ||
| if modFuncErr != nil { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
bool is redundant. The empty string indicates that no closest match was found.