git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Aguilar <davvid@gmail.com>
To: Ferry Huberts <ferry.huberts@pelagic.nl>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	"Shawn O. Pearce" <spearce@spearce.org>
Subject: Re: [PATCH v2 1/2] Ensure consistent usage of mergetool.keepBackup in git
Date: Thu, 9 Apr 2009 20:27:32 -0700	[thread overview]
Message-ID: <20090410032731.GA1545@gmail.com> (raw)
In-Reply-To: <f6297e57a23dc3abac2fcedceb00cecde607de02.1239291673.git.ferry.huberts@pelagic.nl>


Hi

On  0, Ferry Huberts <ferry.huberts@pelagic.nl> wrote:
> In several places merge.keepBackup is used i.s.o.
> mergetool.keepBackup. This patch makes it all
> consistent for git
> 
> Signed-off-by: Ferry Huberts <ferry.huberts@pelagic.nl>
> ---
>  contrib/difftool/git-difftool.txt |    2 +-
>  git-mergetool.sh                  |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)


You missed the usage of merge.keepBackup in
contrib/difftool/git-difftool-helper.

But.. (for better or worse) the "keep backup" feature
was completely removed from difftool, so patching this for
difftool now isn't very useful.

See 2e8af7e4 which is currently in origin/pu:

	difftool: remove the backup file feature


Also, we just got through a very large round of
mergetool/difftool changes.  The latest version is sitting in
Junio's pu branch.  The "da/difftool" branch's head is
currently pointing at 21d0ba7e.

It might be worth basing your work on top of that series since
that'd make things much easier to merge.


My only other comment is:

Aside from git-gui, there are other scripts out there that
use merge.keepBackup instead of mergetool.keepBackup,
so changing the name of the config variable has negligable
user benefit and will cause problems for people that:

A) already have merge.keepBackup configured and then get
surprised when one day all of a sudden git starts leaving
around these ".orig turds" (technical term, overheard from an
actual user) for no apparent reason even though they had
already configured it to do otherwise in the past, or

B) have written GUIs or scripts that know about
merge.keepBackup.

Aside from the naming, there's little user benefit to this
change in my opinion.

Anyways, just my thoughts.

-- 

	David

  reply	other threads:[~2009-04-10  3:30 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-07 15:33 [[PATCH 1/1] Ensure consistent usage of mergetool.keepBackup Ferry Huberts
2009-04-09 15:30 ` RESEND [PATCH " Shawn O. Pearce
2009-04-09 15:45   ` [PATCH v2 1/2] Ensure consistent usage of mergetool.keepBackup in git Ferry Huberts
2009-04-10  3:27     ` David Aguilar [this message]
2009-04-10  6:58       ` Ferry Huberts (Pelagic)
2009-04-10  7:43         ` David Aguilar
2009-04-10  8:18           ` David Aguilar
2009-04-10  8:25             ` Ferry Huberts (Pelagic)
2009-04-10 14:48             ` Markus Heidelberg
2009-04-11 12:04             ` Charles Bailey
2009-04-09 15:45   ` [PATCH v2 2/2] Ensure consistent usage of mergetool.keepBackup in git-gui Ferry Huberts

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=20090410032731.GA1545@gmail.com \
    --to=davvid@gmail.com \
    --cc=ferry.huberts@pelagic.nl \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=spearce@spearce.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).