Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: > The reason I suggested re-purposing `add_opts.keep_locked` is to avoid > polluting that structure with members having overlapping meaning, thus > reducing the confusion-factor for clients of that structure (assuming > that a tri-state `keep_locked` is indeed less confusing). That doesn't > preclude adding a new variable or two local to the `add()` function to > facilitate keeping `add_opts` pure. For instance, it might be as > simple as the below patch. True. It is less trivial to construct the command line option parser so that --reason=<why> and --lock can be given in any order (e.g. they no longer can be a simple OPT_BOOL and OPT_STRING that can be given independently but needs some postprocessing like your patch did), but it is not rocket science and keeping add_opts struct leaner will reduce the risk of runtime confusion, I would think, but at the same time, the room for runtime confusion would probably be minor to begin with---so I am fine, if the coder cannot cleanly write the option parser to do so, with the code as posted.
