Conversation
|
@alexjurkiewicz, how does this look to you? |
f7b6c50 to
68c893d
Compare
mypy/main.py
Outdated
| strict_flag_names = [] # type: List[str] | ||
| strict_flag_assignments = [] # type: List[Tuple[str, bool]] | ||
|
|
||
| def add_invertable_flag(flag, *, inverse, default, dest=None, help, strict_flag=False): |
There was a problem hiding this comment.
nit: s/invertable/invertible/
|
I like it! Does this work if I run mypy with |
|
Oops! Fixed. |
|
OK, that logic is much cleaner :) Regarding the param names, I am not a huge fan of a non-standard approach to option inverses (allow -> disallow, foo -> no-foo, fast -> old, more?). I think it will make understanding any given commandline much harder for those not intimately familiar with the mypy options. Linters generally use a symbolic approach like Either of those I prefer. Beyond that, 👍 and thanks 🙌 ! |
|
Because the opt-out names aren't standard, they may need to be documented somehow. Users probably can't guess that the opposite of I was going to suggest adding the inverse to the end of the argument's help text (perhaps only if it's a strict option), but this might be too verbose. No other good ideas immediately... |
|
To my eye "allow"<->"disallow" and "show"<->"hide" feel very predictable and natural, but "check"<->"ignore" much less so. I think it may be because "allow"<->"disallow" is adding/removing an explicit negation, and "show"<->"hide" is a very common opposition in a wide range of UIs, while "check" vs. "ignore" isn't a standard pairing. Especially when looking at "ignore", there are a lot of possible opposites for that and it isn't obvious that the opposite will be "check". I don't think I'd be sure even looking at the pair "ignore" and "check", in say two different scripts or a CLI invocation that was overriding a config file, that they were exact opposites and there wasn't some third alternative. Perhaps |
|
I agree that check/ignore and fast/old aren't obvious opposites. I'll change both of those to use the generic This PR does document the inverse flags, actually -- it adds them to the help output of the standard flag. E.g.: |
57dba98 to
2e75c80
Compare
|
@alexjurkiewicz does this seems reasonable to you? If so, I'll merge. |
|
Ah, my understanding is that you're on vacation. We'd like to have the inverted options, so I'm going to merge this (and my understanding is that the This PR hasn't received detailed review, but enough people have looked at it (and it's an isolated-enough part of the code) that the merge feels reasonable. |
|
Yes, I am away, my email reply went AWOL apparently. Thanks for helping out / rewriting this feature! I really appreciate the assistance 👍 |
|
Glad I could help! |
Alternative to #2700 with less boilerplate. Fixes #2585.