Re: [PATCH 3/3] diff.c: simplify diff_opt_break_rewrites()

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 22, 2019, 23:18 UTC
Message-ID: <xmqqpnsocmdh.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <xmqqtvi0e7sv.fsf@gitster-ct.c.googlers.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 6 quoted lines

> It would make it even easier to follow if you did
>
> 	if (*arg == '/') {
> 		opt2 = ...;
> 		arg++;
> 	} else {

Oops, this should read "else if (!*arg) {", of course, to match the original. Sorry for the noise.

> 		opt2 = 0;
> 	}

And then we'd want a blank line here to make it clear that the previous if/else cascade has finished, and the error checking we see next is not part of it.

Show 6 quoted lines

> 	if (*arg)
> 		return error(...);
>
> It makes it clear that opt2==0 means <n> form and not <n>/<m> form,
> by having an explicit assignment while we parse what *arg points at,
> without the initialization to 0 in the variable definition.