Re: [PATCH 0/4] diff --cc: relax path filtering

2015-04-03 Thread Max Kirillov
On Thu, Apr 02, 2015 at 05:13:10PM -0400, Jeff King wrote:
 On Thu, Apr 02, 2015 at 11:34:09PM +0300, Max Kirillov wrote:
 
 For diff --cc, paths fitering used to select only paths which have
 changed in all parents, while diffing itself output hunks which are
 changed in as few as 2 parents.
 
 I'm confused about used to here. Is this a regression due to the
 combine-diff rewriting that happened in 2.0, or do you just mean before
 this patch series, we used to do this other thing.

As far as I can see it was always, at least since 1.8.0;
the test script did not work before that.

 Fix intersect_paths() to add paths which have at least 2 changed
 parents.
 
 I'd worry a little that this is increasing the cost to do log --cc, as
 it means we will have to open and look at extra files, and we may find
 in many cases that there aren't any interesting hunks. Which would imply
 we might want to put it behind a flag, rather than as the default
 (--cc-me-harder).
 
 But if I'm understanding the issue correctly, this should only matter
 for octopus merges.  That is, the old rule for looking at a path was is
 there at least one parent whose content we took verbatim, but the new
 one is are there are at least 2 parents whose content we did not take
 verbatim. With only two parents, those would be the same thing, I
 think.

Yes, I hope so. I tried to reproduce benchamrk which is in 8518ff8fabc
(git log --raw --no-abbrev --no-renames (-c|--cc) v3.10..v3.11),
and saw no difference. But my times was about 3 seconds, not
20 as there, andI cannot say my computer is very fast, so
probably I've done something wrong.

-- 
Max
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/4] diff --cc: relax path filtering

2015-04-02 Thread Max Kirillov
For diff --cc, paths fitering used to select only paths which have
changed in all parents, while diffing itself output hunks which are
changed in as few as 2 parents.

Fix intersect_paths() to add paths which have at least 2 changed
parents.

Intersects with branch 'bc/object-id' which is not yet in master. This is
rebased on top of it.

Max Kirillov (4):
  Add test for showing discarded changes with diff --cc
  combine-diff.c: refactor: extract insert_path()
  diff --cc: relax too strict paths picking
  t4059: rewrite to be adaptive to hunk filtering

 combine-diff.c  |  95 ++---
 t/t4059-diff-merge-with-base.sh | 132 
 2 files changed, 193 insertions(+), 34 deletions(-)
 create mode 100755 t/t4059-diff-merge-with-base.sh

-- 
2.3.4.2801.g3d0809b

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/4] diff --cc: relax path filtering

2015-04-02 Thread Jeff King
On Thu, Apr 02, 2015 at 11:34:09PM +0300, Max Kirillov wrote:

 For diff --cc, paths fitering used to select only paths which have
 changed in all parents, while diffing itself output hunks which are
 changed in as few as 2 parents.

I'm confused about used to here. Is this a regression due to the
combine-diff rewriting that happened in 2.0, or do you just mean before
this patch series, we used to do this other thing.

 Fix intersect_paths() to add paths which have at least 2 changed
 parents.

I'd worry a little that this is increasing the cost to do log --cc, as
it means we will have to open and look at extra files, and we may find
in many cases that there aren't any interesting hunks. Which would imply
we might want to put it behind a flag, rather than as the default
(--cc-me-harder).

But if I'm understanding the issue correctly, this should only matter
for octopus merges.  That is, the old rule for looking at a path was is
there at least one parent whose content we took verbatim, but the new
one is are there are at least 2 parents whose content we did not take
verbatim. With only two parents, those would be the same thing, I
think.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html