-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
feat: add TextFlag #2055
base: main
Are you sure you want to change the base?
feat: add TextFlag #2055
Conversation
* Added `TextFlag` which supports setting values for types that satisfies the `encoding.TextMarshaller` and `encoding.TextUnmarshaller` which is very handy when you want to set log levels or string-like types that satifies the interfaces. Fixes: urfave#2051 Signed-off-by: Tobias Dahlberg <[email protected]>
Signed-off-by: Eng Zer Jun <[email protected]>
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.
Thanks for your contribution! 😊
Just a few comments regarding the tests. If possible, please add a few more test cases to cover edge cases and improve coverage.
return err | ||
} | ||
|
||
if slices.Compare(text, []byte("INFO")) != 0 { |
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.
if slices.Compare(text, []byte("INFO")) != 0 { | |
if !slices.Equal(text, []byte("INFO")) { |
if err := tt.flag.Apply(set); err != nil { | ||
t.Fatalf("Apply(%v) failed: %v", tt.args, err) | ||
} | ||
|
||
err := set.Parse(tt.args) | ||
if (err != nil) != tt.wantErr { | ||
t.Errorf("Parse() error = %v, wantErr %v", err, tt.wantErr) | ||
|
||
return | ||
} else if (err != nil) == tt.wantErr { | ||
// Expected error. | ||
return | ||
} | ||
|
||
if got := tt.flag.GetValue(); got != tt.want { | ||
t.Errorf("Value = %v, want %v", got, tt.want) | ||
} |
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.
Use the testify package for assertions
TextFlag
which supports setting values for types that satisfies bothencoding.TextMarshaller
andencoding.TextUnmarshaller
which is very handy when you want to set log levels or string-like types that satisfies the interfaces.Fixes: #2051
What type of PR is this?
What this PR does / why we need it:
Making it easier to set types that satisfies
encoding.TextMarshaller
andencoding.TextUnmarshaller
such aslog/slog.LogLevelVar
, using flags as see in the standard library'sflag
.Which issue(s) this PR fixes:
Fixes #2051
Release Notes