Re: [PATCH 2/8] tag: Refactor the options handling code to be less bizarro

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Lars Hjemli <hjemli@gmail.com>, Jeff King <peff@peff.net>,
	Christian Couder <christian.couder@gmail.com>,
	Carlos Rica <jasampler@gmail.com>,
	Samuel Tardieu <sam@rfc1149.net>,
	Tom Grennan <tmgrennan@gmail.com>
Subject: Re: [PATCH 2/8] tag: Refactor the options handling code to be less bizarro
Date: Sat, 18 Mar 2017 21:00:50 +0100	[thread overview]
Message-ID: <CACBZZX7G+fj+Fr1ex-vAD47yzvpJXma5YDMBrMoffr_KWFFx7w@mail.gmail.com> (raw)
In-Reply-To: <xmqqo9wy1j3s.fsf@gitster.mtv.corp.google.com>

On Sat, Mar 18, 2017 at 8:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> But I thought it was very a very bizarre pattern to set us to cmdmode
>> = 'l' when we're not in that mode at all just to, as can be seen in
>> the diff, get around a slightly more verbose one-time if-check.
>
> When I wrote my response, I viewed that setting as committing to be
> in the "list" mode, not as a workaround.  So checking with !cmdmode
> to make sure that the command line is not asking to create a tag
> makes tons of sense; the new test makes it unreadable from that
> point of view.

Makes sense. Looking at this more carefully there's never any cases
where `create_tag_object && cmdmode == 'l'` is true once we make it
past `usage_with_options`, which was the bug I introduced locally
which made this whole thing a bit confusing for me.

I'll drop this patch from v2.