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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 27, 2008, 16:24 UTC
Message-ID: <7v7ifob0et.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080327073043.GB22444@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 11 quoted lines

> When a path is examined in the patch subcommand, any mode
> changes in the file are given to use in the diff header by
> git-diff. If no hunks are staged, then we throw out that
> header. But if _any_ hunks are staged, we use the header,
> and the mode is changed.
>
> Since the 'p'atch command should just be dealing with hunks
> that are shown to the user, it makes sense to just ignore
> mode changes entirely. We do squirrel away the mode, though,
> since a future patch will allow users to select the mode
> update.

I agree that treating each logical block of the metainformation as if it is a hunk on its own makes sense. Possibly, it would be useful to make the interactive patch session look like this:

    diff --git a/gostak b/doshes
    old mode 100644
    new mode 100755
    Stage the mode change [y/n/a/d/j/J/?]?
    similarity index 90%
    rename from gostak
    rename to doshes
    Stage the name change [y/n/a/d/j/J/?]?
    @@ -1,5 +1,5 @@
     The
    -gostak
    +Gostak
     distims
     doshes
    Stage this hunk [y/n/a/d/j/J/?]?

Handlilng the rename needs a bit more thought and enhancements not just in your parse_diff_header() code but the way we extract the diff text (we would need to first find renames by whole-tree diff and then feed two (src,dst) paths in parse_diff() to obtain the text), but it should be doable.

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