All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, "Jörg Sommer" <joerg@alea.gnuu.de>,
	"Wincent Colaiuta" <win@wincent.com>
Subject: Re: [PATCH 1/2] add--interactive: ignore mode change in 'p'atch command
Date: Thu, 27 Mar 2008 11:29:58 -0700	[thread overview]
Message-ID: <7vprtg9g0p.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <20080327171022.GA27189@coredump.intra.peff.net> (Jeff King's message of "Thu, 27 Mar 2008 13:10:23 -0400")

Jeff King <peff@peff.net> writes:

> 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.

Yeah, but the user is really into microcommits, like "separate mode
change" thing really matters, maybe the user would want to make three
commits (1) chmod +x, (2) pure rename, and (3) content changes.

I personally think that is not worth it, so I am agreeing with you on the
"rename" one.

Even though your two patches make perfect sense at the philosophical level
and I very much like it, I doubt "separating mode change" is so useful
from the practical point of view for that matter.

I would even imagine that, if we did not have "add -p" before, and the
very initial implementation of it had started from the behaviour your two
patches bring in, and then a patch came to make it the current "not asking
about mode change separately and if the user chooses to add anything from
the patch hunks, stage the mode change along with it" behaviour, people
might even think that such a patch is an improvement in usability by
asking one less question.  I dunno.

>> 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.

That I'd agree with very much.  Thanks for clarificaiton.

  reply	other threads:[~2008-03-27 18:30 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1206602393.git.peff@peff.net>
2008-03-27  7:30 ` [PATCH 1/2] add--interactive: ignore mode change in 'p'atch command Jeff King
2008-03-27 16:24   ` Junio C Hamano
2008-03-27 17:10     ` Jeff King
2008-03-27 18:29       ` Junio C Hamano [this message]
2008-03-27 19:31         ` Jeff King
     [not found]         ` <m3fxucuesq.fsf@localhost.localdomain>
2008-03-27 22:16           ` Jeff King
2008-03-27  7:32 ` [PATCH 2/2] add--interactive: allow user to choose mode update Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vprtg9g0p.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=joerg@alea.gnuu.de \
    --cc=peff@peff.net \
    --cc=win@wincent.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.