go-flags should detect invalid API usage/user error at init time, refuse to work.

[dmitshur]

Originally reported in #80 (comment) (/cc @slimsag) but it's inside a closed issue, and didn't seem to get a response.

The issue is that if a user accidentally uses default:"true" on a bool flag, they will have a variable that is unfit to be a flag since it can only ever have the single value true, and there's no way to make it false.

Since it's possible to detect such invalid usage at runtime, it'd be better to treat it as a fatal error rather than letting it go unnoticed.

---

On another note, while I agree with the rationale mentioned in #80 (comment) that ideally bool values should always start out false and adding them should toggle the value to true, it's also the case that sometimes it might be desirable to temporarily change the default value of a boolean value without having to change the CLI name/logic/API (/cc @renfredxh). I'm not completely sure it's a great idea, but it's worth considering.

[jessevdk]

I'm not fond of making things fatal inside a library. E.g. maybe you temporarily set the default value to true for debugging. It might not always be an error. I think this is a really minor issue, and I've not heard anyone else complain about this. I'm inclined to close the issue.

[dmitshur]

a variable that is unfit to be a flag since it can only ever have the single value true, and there's no way to make it false.

I think this is a really minor issue

So you think a flag that has a value that can't be changed is a really minor issue? I just want to confirm you understand what the issue is. It seems pretty significant to me, but it is rare.

E.g. maybe you temporarily set the default value to true for debugging.

And you might accidentally leave it that way, and not realize users of your binary will not be able to change the flag value.

it'd be better to treat it as a fatal error rather than letting it go unnoticed.

Maybe a log.Println("this flag is effectively a const, its value can not be changed by users") warning if you think a fatal error is not appropriate.

The issue is a potentially unusable flag that can be unnoticed (as has happened in the situation where I ran into this; the developer did not realize there was an issue, and as a user, I could not change the value of the flag at all).

[dmitshur]

Of course, if you don't think this is an issue, you're welcome to close this. I only reported it because I wanted to make a suggestion of what I thought would be an improvement to this library, but given that I'm aware of this caveat by now, it doesn't affect me.

[dmitshur]

Thanks for resolving this issue! That looks like a good solution. 👍

[aakso]

Just a note: this change broke my code. There was a way to change the default value: thru env vars.

Anyways, I'm just going to change my defaults but I feel that one should consider all the angles when implementing breaking changes.