Re: [PATCH v3] submodule: port subcommand 'set-url' from shell to C

From: Shourya Shukla <shouryashukla.oo@gmail.com>
Date: May 4, 2020, 17:39 UTC
Message-ID: <20200504173936.GA8919@konoha>
In-Reply-To: <xmqqd07jitjz.fsf@gitster.c.googlers.com>

On 04/05 08:55, Junio C Hamano wrote:

Show 10 quoted lines

> > +	argc = parse_options(argc, argv, prefix, set_url_options,
> > +			     usage, 0);
> > +
> > +	if (quiet)
> > +		quiet |= OPT_QUIET;
> 
> This is bogus.  "command --quiet --quiet" would count-up quiet twice
> and would make it 2, and you or OPT_QUIET==1 in to make it 3, but
> your intention is quite clear that you want to pass 1 to
> sync_submodule() in such a case.

This is a grave mistake from my side. Though I do not understand how will `quiet` be counted twice. Let's say we pass `--quiet` option in the command, then `quiet` is set to 1. And now if we do `quiet|= OPT_QUIET`, this will make `quiet` 1 again right? Could you please tell me what am I missing out here?

The fix you suggested (quiet ? OPT_QUIET : 0), we use this because we want to ensure `quiet` goes into sync as either 1/0 right? Not any other non-zero positive integer right?