From: Jeff King <peff@peff.net>
Date: Thu, 27 Mar 2008 17:10:23 GMT
Subject: Re: [PATCH 1/2] add--interactive: ignore mode change in 'p'atch command
Message-ID: <20080327171022.GA27189@coredump.intra.peff.net>
In-Reply-To: <7v7ifob0et.fsf@gitster.siamese.dyndns.org>

On Thu, Mar 27, 2008 at 09:24:10AM -0700, Junio C Hamano wrote:

>     similarity index 90%
>     rename from gostak
>     rename to doshes
>     Stage the name change [y/n/a/d/j/J/?]?

I hadn't thought about renames. But I wonder if it really makes sense in
the context of a single path. If I have content in "doshes", what does
it matter at this point that it came from "gostak"? IOW, what does
saying 'y' here really do? What is the workflow around it?

> By the way, why was it done as a new sub called from parse_diff() and not
> as a part of parse_diff() itself?

Code clarity.

The parsing code seemed less convoluted to me that way. parse_diff is
about linearly splitting the input into hunks. The first hunk just
happens to be "everything before the first patch hunk".

But splitting head versus mode requires non-linear parsing. Doing it in
parse_diff would require adding some state to the loop. It seemed
more readable to me to compose it from two simple loops rather than one
more complex loop.

I can do it the other way if you prefer.

-Peff