[go] flag: add All iterators and Flag.IsSet
qiu laidongfeng (Gerrit)
qiu laidongfeng has uploaded the change for review![]( "Open in Gerrit")
Commit message
flag: add All iterators and Flag.IsSet
Fixes #65675
Change diff
diff --git a/src/flag/flag.go b/src/flag/flag.go
index 71902f7..2bc7416 100644
--- a/src/flag/flag.go
+++ b/src/flag/flag.go
@@ -87,6 +87,7 @@
"errors"
"fmt"
"io"
+ "iter"
"os"
"reflect"
"runtime"
@@ -410,6 +411,7 @@
Usage string // help message
Value Value // value as set
DefValue string // default value (as text); for usage message
+ IsSet bool
}
// sortFlags returns the flags as a slice in lexicographical sorted order.
@@ -459,6 +461,22 @@
}
}
+// All yields the flags in lexicographical order.
+func (f *FlagSet) All() iter.Seq[*Flag] {
+ return func(yield func(*Flag) bool) {
+ for _, flag := range sortFlags(f.formal) {
+ if !yield(flag) {
+ break
+ }
+ }
+ }
+}
+
+// All yields all command-line flags, in lexicographical order.
+func All() iter.Seq[*Flag] {
+ return CommandLine.All()
+}
+
// VisitAll visits the command-line flags in lexicographical order, calling
// fn for each. It visits all flags, even those not set.
func VisitAll(fn func(*Flag)) {
@@ -1016,7 +1034,7 @@
}
// Remember the default value as a string; it won't change.
- flag := &Flag{name, usage, value, value.String()}
+ flag := &Flag{name, usage, value, value.String(), false}
_, alreadythere := f.formal[name]
if alreadythere {
var msg string
@@ -1114,6 +1132,7 @@
}
return false, f.failf("flag provided but not defined: -%s", name)
}
+ flag.IsSet = true
if fv, ok := flag.Value.(boolFlag); ok && fv.IsBoolFlag() { // special case: doesn't need an arg
if hasValue {
diff --git a/src/flag/flag_test.go b/src/flag/flag_test.go
index 278ae7e..da6f77c 100644
--- a/src/flag/flag_test.go
+++ b/src/flag/flag_test.go
@@ -190,6 +190,11 @@
if err := f.Parse(args); err != nil {
t.Fatal(err)
}
+ for tmp := range f.All() {
+ if !tmp.IsSet {
+ t.Errorf("flag %s should be set, but is not", tmp.Name)
+ }
+ }
if !f.Parsed() {
t.Error("f.Parse() = false after Parse")
}
Change information
- M src/flag/flag.go
- M src/flag/flag_test.go
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Alan Donovan added 2 comments![]( "Open in Gerrit")
// All yields the flags in lexicographical order.Let's keep this part:
// It visits all flags, even those not set.
Ditto at func All.
flag.IsSet = trueThere's an inconsistency between `flag.IsSet` and membership of `f.actual[flag]` that we should avoid. Given that we cannot change f.actual at this stage, I think that means we must interpret IsSet as "Flag.Set was called successfully" (and document IsSet as such). That means this statement should be moved to L1161.
Could you first add a test case that asserts this consistency, and fails with the current patchset? Thanks.
Related details
- Rob Pike
- qiu laidongfeng
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Alan Donovan added 1 comment![]( "Open in Gerrit")
In addition to the test of the `composite` analyzer you fixed in x/tools, there's another little test in src/cmd/vet/testdata/composite/composite.go that needs a similar tweak.
qiu laidongfeng (Gerrit)
qiu laidongfeng uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result-1 by golang...@luci-project-accounts.iam.gserviceaccount.com
Related details
- Rob Pike
- qiu laidongfeng
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
qiu laidongfeng (Gerrit)
qiu laidongfeng voted and added 3 comments![]( "Open in Gerrit")
Votes added by qiu laidongfeng
| Commit-Queue | +1 |
In addition to the test of the `composite` analyzer you fixed in x/tools, there's another little test in src/cmd/vet/testdata/composite/composite.go that needs a similar tweak.
Done
Let's keep this part:
// It visits all flags, even those not set.
Ditto at func All.
Done
There's an inconsistency between `flag.IsSet` and membership of `f.actual[flag]` that we should avoid. Given that we cannot change f.actual at this stage, I think that means we must interpret IsSet as "Flag.Set was called successfully" (and document IsSet as such). That means this statement should be moved to L1161.
Could you first add a test case that asserts this consistency, and fails with the current patchset? Thanks.
- Alan Donovan
- Rob Pike
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Alan Donovan added 4 comments![]( "Open in Gerrit")
Add iterator-style APIs.Release notes use prose:
The new [FlagSet.All] method returns an iterator over all the flags in the FlagSet;
the function [All] does the same for the global [CommandLine] flag set.
The new [Flag.IsSet] field indicates whether the flag has been set.
flag.IsSet = trueLet's move this to L544 so that we keep Set and IsSet together, and the two operations on f.actual together.
flag.IsSet = trueditto, move to L1163.
(It's tempting to factor the common tails of Set + IsSet + f.actual into a function, but the error messages from Set have been carefully specialized here, so best not.)
Related details
- Rob Pike
- qiu laidongfeng
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
qiu laidongfeng (Gerrit)
qiu laidongfeng uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result+1 by golang...@luci-project-accounts.iam.gserviceaccount.com
Related details
- Rob Pike
- qiu laidongfeng
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
qiu laidongfeng (Gerrit)
qiu laidongfeng voted and added 3 comments![]( "Open in Gerrit")
Votes added by qiu laidongfeng
| Commit-Queue | +1 |
3 comments
Release notes use prose:
The new [FlagSet.All] method returns an iterator over all the flags in the FlagSet;
the function [All] does the same for the global [CommandLine] flag set.
The new [Flag.IsSet] field indicates whether the flag has been set.
Done
Let's move this to L544 so that we keep Set and IsSet together, and the two operations on f.actual together.
Done
ditto, move to L1163.
(It's tempting to factor the common tails of Set + IsSet + f.actual into a function, but the error messages from Set have been carefully specialized here, so best not.)
- Alan Donovan
- Rob Pike
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alan Donovan (Gerrit)
Alan Donovan voted![]( "Open in Gerrit")
- Rob Pike
- qiu laidongfeng
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |