Re: [PATCH] commit-tree: utilize parse-options api

[Jeff King <peff@peff.net>]

On Wed, Feb 27, 2019 at 10:46:49PM -0400, Brandon Richardson wrote:

> > If we are going to go this route, I think you might actually want macros
> > that take both "unset" and "args" and make sure that we're not in a
> > situation the callback doesn't expect (e.g., "!unset && !arg"). That
> > lets us continue to declare those at the top of the callback.
> 
> In doing a quick search, I found a fair number instances of this:
> ...
> BUG_ON_OPT_NEG(unset);
> 
> if (!arg)
>      return -1;
> ...

Those are probably my fault. The originals guarded against an unexpected
"unset" by checking "!arg" and returning an error. But it made the
compiler's -Wunused-parameter complain, so I added the BUG_ON_OPT_NEG()
calls as an assertion. At that point the "if (!arg)" could never
trigger, and could have been removed.

> So a macro like this could be useful. I've also found a few instances of this:
> 
> BUG_ON_OPT_NEG(unset);
> BUG_ON_OPT_ARG(arg);

These ones are different. The second one is checking that "arg" _is_
NULL (i.e., we expect that the options struct provided the right flag to
disallow an argument). And that's orthogonal to the unset flag, so it
would not be right to conflate the two in a single macro.

-Peff