Re: [PATCH 1/2] add--interactive: ignore mode change in 'p'atch command

From: Jeff King <peff@peff.net>
Date: Mar 27, 2008, 17:10 UTC
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