git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: "Eckhard Maaß" <eckhard.s.maass@googlemail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Ben Peart <peartben@gmail.com>,
	Ben Peart <Ben.Peart@microsoft.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	"peff@peff.net" <peff@peff.net>,
	"pclouds@gmail.com" <pclouds@gmail.com>,
	"vmiklos@frugalware.org" <vmiklos@frugalware.org>,
	Kevin Willford <kewillf@microsoft.com>,
	"Johannes.Schindelin@gmx.de" <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH v3 2/3] merge: Add merge.renames config setting
Date: Mon, 30 Apr 2018 09:54:10 -0700	[thread overview]
Message-ID: <CABPp-BF3_gkuyS+9QOjoKHidKp=S6LzzyF7MJ06fKah6hE=pDw@mail.gmail.com> (raw)
In-Reply-To: <20180430080341.GA28348@esm>

Hi Eckhard,

On Mon, Apr 30, 2018 at 1:03 AM, Eckhard Maaß
<eckhard.s.maass@googlemail.com> wrote:
> On Fri, Apr 27, 2018 at 01:23:20PM -0700, Elijah Newren wrote:
>> I doubt it has ever been discussed before this thread.  But, if you're
>> curious, I'll try to dump a few thoughts.
>
> Thank you, I try to dump some of mine, too. Maybe let me first stress
> that for me copy detection without --find-copies-harder is much more a
> "find content extracted" (like methods being factored out). In a way
> this is nearer to a rename than to a real copy.

Ooh, if you wanted to detect movement of code between files (as blame
does, I think searches for PICKAXE_BLAME_MOVE would point you in the
right direction) and then try to use that during merge to allow
applied changes to move with the code, that would be awesome.
Expensive, and might be a lot of work to wire it all up, but it'd be
very interesting.  I was only discussing DIFF_DETECT_COPY in my
previous email, which was all about duplicating entire files; that's
something I don't see utility in right now for resolving merges.

<snip>
> I admit that a "real" copy would get unnoticed that way. But the
> semantics of such a copy isn't too clear for me either - did I copy the
> other part to make it independent of the other or did I just employ a
> copy and paste tactic? The former does not want the changes, the later
> does. But I am happy catering to the former here.

Right, if you have to assume that the copy was made to make the code
independent, then there's no value for merge resolution to having
detected the copy in the first place.  That has the advantage of
side-stepping the possible new edge and corner cases I mentioned in
the rest of my email, but it means we shouldn't even spend time
detecting copies -- whether whole file (via DIFF_DETECT_COPY) or
individual lines (via PICKAXE_BLAME_COPY and variants).


Elijah

  reply	other threads:[~2018-04-30 16:54 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-20 13:36 [PATCH v1 0/2] add additional config settings for merge Ben Peart
2018-04-20 13:36 ` [PATCH v1 1/2] merge: Add merge.renames config setting Ben Peart
2018-04-20 17:02   ` Elijah Newren
2018-04-20 17:26     ` Elijah Newren
2018-04-23 12:57       ` Ben Peart
2018-04-20 17:59     ` Ben Peart
2018-04-20 18:34       ` Elijah Newren
2018-04-21  4:23         ` Junio C Hamano
2018-04-23 16:00           ` Ben Peart
2018-04-23 23:23             ` Junio C Hamano
2018-04-24 11:58               ` Johannes Schindelin
2018-04-24 17:47                 ` Elijah Newren
2018-04-25  8:20                   ` Johannes Schindelin
2018-04-22 12:07         ` Eckhard Maaß
2018-04-23 13:15           ` Ben Peart
2018-04-23 21:32             ` Eckhard Maaß
2018-04-24 16:53               ` Ben Peart
2018-04-23 13:22         ` Ben Peart
2018-04-20 13:36 ` [PATCH v1 2/2] merge: Add merge.aggressive " Ben Peart
2018-04-20 17:22   ` Elijah Newren
2018-04-24 16:45     ` Ben Peart
2018-04-24 17:36       ` Elijah Newren
2018-04-24 23:57       ` Junio C Hamano
2018-04-25 14:47         ` Ben Peart
2018-04-20 17:34 ` [PATCH v1 0/2] add additional config settings for merge Elijah Newren
2018-04-20 18:19   ` Ben Peart
2018-04-24 17:11 ` [PATCH v2 " Ben Peart
2018-04-24 17:11   ` [PATCH v2 1/2] merge: Add merge.renames config setting Ben Peart
2018-04-24 18:11     ` Elijah Newren
2018-04-24 18:59     ` Elijah Newren
2018-04-24 20:31       ` Ben Peart
2018-04-25 16:01         ` Elijah Newren
2018-04-24 17:11   ` [PATCH v2 2/2] merge: Add merge.aggressive " Ben Peart
2018-04-25  0:13   ` [PATCH v2 0/2] add additional config settings for merge Junio C Hamano
2018-04-25 15:22     ` Ben Peart
2018-04-26  1:48       ` Junio C Hamano
2018-04-26 20:52 ` [PATCH v3 0/3] add merge.renames config setting Ben Peart
2018-04-26 20:52   ` [PATCH v3 1/3] merge: update documentation for {merge,diff}.renameLimit Ben Peart
2018-04-26 23:11     ` Elijah Newren
2018-04-26 23:23       ` Jonathan Tan
2018-04-26 20:52   ` [PATCH v3 2/3] merge: Add merge.renames config setting Ben Peart
2018-04-26 22:52     ` Elijah Newren
2018-04-27  0:54       ` Ben Peart
2018-04-27  2:23         ` Junio C Hamano
2018-04-27  3:28           ` Elijah Newren
2018-04-27  7:23             ` Johannes Schindelin
2018-04-27 14:32               ` Elijah Newren
2018-04-27 18:37           ` Eckhard Maaß
2018-04-27 20:23             ` Elijah Newren
2018-04-30  8:03               ` Eckhard Maaß
2018-04-30 16:54                 ` Elijah Newren [this message]
2018-04-27  4:17         ` Elijah Newren
2018-04-27 18:19         ` Elijah Newren
2018-04-30 13:11           ` Ben Peart
2018-04-30 16:12             ` Re: Elijah Newren
2018-05-02 14:33               ` Re: Ben Peart
2018-04-26 20:52   ` [PATCH v3 3/3] merge: pass aggressive when rename detection is turned off Ben Peart
2018-04-26 23:00     ` Elijah Newren
2018-04-26 22:08   ` [PATCH v3 0/3] add merge.renames config setting Elijah Newren
2018-05-02 16:01 ` [PATCH v4 0/3] add additional config settings for merge Ben Peart
2018-05-02 16:01   ` [PATCH v4 1/3] merge: update documentation for {merge,diff}.renameLimit Ben Peart
2018-05-02 16:01   ` [PATCH v4 2/3] merge: Add merge.renames config setting Ben Peart
2018-05-04  3:07     ` Junio C Hamano
2018-05-02 16:01   ` [PATCH v4 3/3] merge: pass aggressive when rename detection is turned off Ben Peart
2018-05-02 17:20   ` [PATCH v4 0/3] add additional config settings for merge Elijah Newren

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='CABPp-BF3_gkuyS+9QOjoKHidKp=S6LzzyF7MJ06fKah6hE=pDw@mail.gmail.com' \
    --to=newren@gmail.com \
    --cc=Ben.Peart@microsoft.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=eckhard.s.maass@googlemail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kewillf@microsoft.com \
    --cc=pclouds@gmail.com \
    --cc=peartben@gmail.com \
    --cc=peff@peff.net \
    --cc=vmiklos@frugalware.org \
    /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).