Git development
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
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 13:10:23 -0400	[thread overview]
Message-ID: <20080327171022.GA27189@coredump.intra.peff.net> (raw)
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

  reply	other threads:[~2008-03-27 17:11 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 [this message]
2008-03-27 18:29       ` Junio C Hamano
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=20080327171022.GA27189@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=joerg@alea.gnuu.de \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox