All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Thomas Rast <trast@student.ethz.ch>
Cc: Nanako Shiraishi <nanako3@lavabit.com>,
	Matt Graham <mdg149@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH] new test fails "add -p" for adds on the top line
Date: Sat, 16 May 2009 12:51:16 -0700	[thread overview]
Message-ID: <7vab5cn7wr.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <7viqk1ndlk.fsf@alter.siamese.dyndns.org> (Junio C. Hamano's message of "Sat\, 16 May 2009 10\:48\:23 -0700")

Junio C Hamano <gitster@pobox.com> writes:

> Thomas Rast <trast@student.ethz.ch> writes:
> ...
>> The above commit still reverts cleanly, but AFAICS merge_hunk blindly
>> trusts the hunk headers, an assumption that is no longer valid due to
>> the 'edit' feature.
>
> Heh, here is my "I told you so" moment ;-).

It never blindly trusted before the edit 'feature'; it counted carefully
and it could do so because it had all the necessary information.

I told you that 'edit' could remember the line offset and line numbers
before giving the buffer to the end user, and then recount and adjust the
count after getting the edited results back, to update the offset and
count with the same carefulness.  You (and I think there was somebody else
who was helping) didn't listen.

Fundamentally, after you remove some hunks (and worse yet, you modify
some) from the patch and feed that to "git apply --recount", it can never
do as thorough a job as you could do inside "add -p" itself.  The latter
has more information (the omitted hunks, and the hunks before/after the
user edited) necessary to reconstruct the line numbers and hunk size.  To
keep the whole process more robust and trustworthy, you must do the
necessary computation while you still have all the information about the
hunks you are not feeding to the downstream.

That was what the "I told you so" was about in my message.

It is not too late to teach the 'edit hack' to do so.  That would allow us
to remove the "$_->{DIRTY}" bit my "how about this" patch adds, and I'll
stop calling it the 'edit hack' and start calling it the 'edit feature'
when that happens ;-).

But at least the "how about this" patch should restore the original
behaviour as long as the user does not use the 'edit hack' for now.

      parent reply	other threads:[~2009-05-16 19:54 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-16  3:10 [PATCH] new test fails "add -p" for adds on the top line Matt Graham
2009-05-16 10:25 ` Nanako Shiraishi
2009-05-16 14:12   ` Thomas Rast
2009-05-16 17:48     ` Junio C Hamano
2009-05-16 17:55       ` Sverre Rabbelier
2009-05-16 19:13         ` Junio C Hamano
2009-05-16 19:14           ` Sverre Rabbelier
2009-05-16 19:51       ` Junio C Hamano [this message]

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=7vab5cn7wr.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=mdg149@gmail.com \
    --cc=nanako3@lavabit.com \
    --cc=trast@student.ethz.ch \
    /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.