pflag icon indicating copy to clipboard operation
pflag copied to clipboard

Confusing semantics of Flag.Changed

Open peter-mogensen opened this issue 3 years ago • 2 comments

The documentation says:

Changed bool // If the user set the value (or if left to default)

But it also says:

// Changed returns true if the flag was explicitly set during Parse() and false
// otherwise
func (f *FlagSet) Changed(name string) bool {
...
}

Both can't be true. - unless it's explicitly the intention that the field Changed and the function Changed has different behaviors.

peter-mogensen avatar Mar 14 '21 07:03 peter-mogensen

"user" in first formulation is somewhat ambiguous — command-line user or programmer using the pflag library? I believe intent here was the former, whether the flag was set explicitly by command-line flag, which matches the function's doc.

Here are subtleties I learned by trial-and-error and code reading:

  • Changed is set even when the value passed on command-line equals the default value. (This is what makes it useful!)

  • pflag has no way to detect direct assignments to the variable associated with a flag, e.g.:

    var flagvar int
    flagset.IntVar(&flagvar, "flagname", ...)
    pflag.Parse()
    ...
    flagvar = 42
    

    .Changed remains false in that case.

  • flag.Value.Set("42") has no effect on .Changed! This is somewhat confusing :-| This is a direct call to the Value interface which each type implements differently. It has no access to the Flag object, so can't touch its .Changed field.

  • flagset.Set("flagname", "42") does set .Changed. :arrow_left: Use this if you want to simulate "as if the user passed it on command line".

cben avatar Apr 03 '21 21:04 cben

Ok ... I think I might have realized the source of my confusion.

The comment: "If the user set the value (or if left to default)" can be interpreted in 2 ways (and I read it wrong).

wrong: The value is true if the user set the value or if left to default right: The value is true if explicitly set - or it's false if left to default.

Also ... you're right that flag.Value.Set() not setting Changed is somewhat confusing. That API is also just meant to be able to define new kind of flags, right ... not to actually set concrete instances of flags in the code.

peter-mogensen avatar May 04 '21 08:05 peter-mogensen