git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jochen Sprickerhof <jochen@sprickerhof.de>
To: phillip.wood@dunelm.org.uk
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH] add -p: coalesce hunks before testing applicability
Date: Mon, 3 Sep 2018 21:01:14 +0200	[thread overview]
Message-ID: <20180903190114.GC17416@vis> (raw)
In-Reply-To: <e5b2900a-0558-d3bf-8ea1-d526b078bbc2@talktalk.net>

[-- Attachment #1: Type: text/plain, Size: 1705 bytes --]

Hi Phillip,

* Phillip Wood <phillip.wood@talktalk.net> [2018-08-30 14:47]:
>When $newhunk is created it is marked as dirty to prevent 
>coalesce_overlapping_hunks() from coalescing it. This patch does not 
>change that. What is happening is that by calling 
>coalesce_overlapping_hunks() the hunks that are not currently selected 
>are filtered out and any hunks that can be coalesced are (I think that 
>in the test that starts passing with this patch the only change is the 
>filtering as there's only a single hunk selected).

Agreed here. It would be enough to include the first hunk in the test to 
make it fail again. Still I would see the patch as going in the right 
direction as we need something like coalesce_overlapping_hunks() to make 
the hunks applicable after the edit.

>This is a subtle change to the test for the applicability of an edited 
>hunk. Previously when all the hunks were used to create the test patch 
>we could be certain that if the test patch applied then if the user 
>later selected any unselected hunk or deselected any selected hunk 
>then that operation would succeed. I'm not sure that is true now (but 
>I haven't thought about it for very long).

I'm not sure here. If we use the same test from t3701, do s(plit), 
y(es), e(dit), it would fail later on. Can you come up with an example?

> We could restore the old 
>test condition and coalesce the hunks by copying all the hunks and 
>setting $hunk->{USE}=1 when creating the test patch if that turns out 
>to be useful (it would be interesting to see if the test still passes 
>with that change).

We set USE=1 for $newhunk already, or where would you set it?

Cheers Jochen

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2018-09-03 19:16 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-28  8:58 [PATCH] add -p: coalesce hunks before testing applicability Jochen Sprickerhof
2018-08-28 18:07 ` Junio C Hamano
2018-08-30 13:47   ` Phillip Wood
2018-08-30 14:51     ` Junio C Hamano
2018-09-03 19:01     ` Jochen Sprickerhof [this message]
2018-09-13 10:20       ` Phillip Wood
2018-09-23 17:16         ` Jochen Sprickerhof
2019-03-22 14:06         ` Johannes Schindelin
2019-06-02 14:17           ` Phillip Wood
2019-06-03 13:40             ` Johannes Schindelin
2019-06-03 14:59               ` Phillip Wood
2019-06-04 13:32                 ` Johannes Schindelin

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=20180903190114.GC17416@vis \
    --to=jochen@sprickerhof.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood@dunelm.org.uk \
    /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;
as well as URLs for NNTP newsgroup(s).