Git development
 help / color / mirror / Atom feed
* [PATCH v2 3/3] branch: mark more strings for translation
From: Nguyễn Thái Ngọc Duy @ 2013-01-25 12:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Matthieu Moy,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1359118225-14356-1-git-send-email-pclouds@gmail.com>

---
 builtin/branch.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index ca61c5b..597b578 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -466,7 +466,7 @@ static void add_verbose_info(struct strbuf *out, struct ref_item *item,
 			     int verbose, int abbrev)
 {
 	struct strbuf subject = STRBUF_INIT, stat = STRBUF_INIT;
-	const char *sub = " **** invalid ref ****";
+	const char *sub = _(" **** invalid ref ****");
 	struct commit *commit = item->commit;
 
 	if (commit && !parse_commit(commit)) {
@@ -590,7 +590,7 @@ static int print_ref_list(int kinds, int detached, int verbose, int abbrev, stru
 		struct commit *filter;
 		filter = lookup_commit_reference_gently(merge_filter_ref, 0);
 		if (!filter)
-			die("object '%s' does not point to a commit",
+			die(_("object '%s' does not point to a commit"),
 			    sha1_to_hex(merge_filter_ref));
 
 		filter->object.flags |= UNINTERESTING;
@@ -854,7 +854,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 		if (!argc) {
 			if (detached)
-				die("Cannot give description to detached HEAD");
+				die(_("Cannot give description to detached HEAD"));
 			branch_name = head;
 		} else if (argc == 1)
 			branch_name = argv[0];
@@ -866,10 +866,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			strbuf_release(&branch_ref);
 
 			if (!argc)
-				return error("No commit on branch '%s' yet.",
+				return error(_("No commit on branch '%s' yet."),
 					     branch_name);
 			else
-				return error("No such branch '%s'.", branch_name);
+				return error(_("No branch named '%s'."),
+					     branch_name);
 		}
 		strbuf_release(&branch_ref);
 
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v2 2/3] branch: give a more helpful message on redundant arguments
From: Nguyễn Thái Ngọc Duy @ 2013-01-25 12:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Matthieu Moy,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1359118225-14356-1-git-send-email-pclouds@gmail.com>

While at there, do not stop user from editing a branch description
when the unrelated HEAD is detached.
---
 builtin/branch.c  | 12 ++++++------
 t/t3200-branch.sh |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 50fcacc..ca61c5b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -852,14 +852,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		const char *branch_name;
 		struct strbuf branch_ref = STRBUF_INIT;
 
-		if (detached)
-			die("Cannot give description to detached HEAD");
-		if (!argc)
+		if (!argc) {
+			if (detached)
+				die("Cannot give description to detached HEAD");
 			branch_name = head;
-		else if (argc == 1)
+		} else if (argc == 1)
 			branch_name = argv[0];
 		else
-			usage_with_options(builtin_branch_usage, options);
+			die(_("cannot edit description of more than one branch"));
 
 		strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
 		if (!ref_exists(branch_ref.buf)) {
@@ -881,7 +881,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		else if (argc == 2)
 			rename_branch(argv[0], argv[1], rename > 1);
 		else
-			usage_with_options(builtin_branch_usage, options);
+			die(_("too many branches for a rename operation"));
 	} else if (new_upstream) {
 		struct branch *branch = branch_get(argv[0]);
 
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 80e6be3..f3e0e4a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -73,8 +73,8 @@ test_expect_success \
 
 test_expect_success \
     'git branch -m dumps usage' \
-       'test_expect_code 129 git branch -m 2>err &&
-	test_i18ngrep "[Uu]sage: git branch" err'
+       'test_expect_code 128 git branch -m 2>err &&
+	test_i18ngrep "too many branches for a rename operation" err'
 
 test_expect_success \
     'git branch -m m m/m should work' \
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* [PATCH v2 1/3] branch: reject -D/-d without branch name
From: Nguyễn Thái Ngọc Duy @ 2013-01-25 12:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Matthieu Moy,
	Nguyễn Thái Ngọc Duy
In-Reply-To: <1359102416-1240-1-git-send-email-pclouds@gmail.com>

---
 builtin/branch.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 873f624..50fcacc 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -837,9 +837,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		colopts = 0;
 	}
 
-	if (delete)
+	if (delete) {
+		if (!argc)
+			die(_("branch name required"));
 		return delete_branches(argc, argv, delete > 1, kinds, quiet);
-	else if (list) {
+	} else if (list) {
 		int ret = print_ref_list(kinds, detached, verbose, abbrev,
 					 with_commit, argv);
 		print_columns(&output, colopts, NULL);
-- 
1.8.0.rc2.23.g1fb49df

^ permalink raw reply related

* Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim
From: Sebastian Schuberth @ 2013-01-25 11:39 UTC (permalink / raw)
  To: David Aguilar; +Cc: Junio C Hamano, git, John Keeping
In-Reply-To: <CAHGBnuMPQWr8Z9jeQmHs7wFN5kf=MwBEEDy_M-QeY0mRnSke_g@mail.gmail.com>

On Fri, Jan 25, 2013 at 11:34 AM, Sebastian Schuberth
<sschuberth@gmail.com> wrote:

>> I thought Git did something sensible there like create a normal file?
>
> It does not. Also see my answer over here:
> http://stackoverflow.com/questions/11412028/git-not-storing-symlink-as-a-symlink-on-windows/11412341#11412341

This one might be the even more appropriate answer:
http://stackoverflow.com/questions/11662868/what-happens-when-i-clone-a-repository-with-symlinks-on-windows/11664406#11664406

-- 
Sebastian Schuberth

^ permalink raw reply

* Re: [PATCH v4 1/4] t6006 (rev-list-format): don't hardcode SHA-1 in expected outputs
From: Alexey Shumkin @ 2013-01-25 11:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v38xqba04.fsf@alter.siamese.dyndns.org>

> Why do we want "whatever_7" variables and use "cut -c1-7" to produce
> them?  Is "7" something we care deeply about?
> 
> I think what we care a lot more than "7" that happens to be the
> current default value is to make sure that, if we ever update the
> default abbreviation length to a larger value, the abbreviation
> shown with --format=%h is consistent with the abbreviation that is
> given by rev-parse --short.
> 
>     head1_short=$(git rev-parse --short $head1)
> 
> perhaps?
> 
> > +	echo changed >foo &&
> > +	git commit -a -m "changed foo" &&
> > +	head2=$(git rev-parse --verify HEAD) &&
> > +	head2_7=$(echo $head2 | cut -c1-7) &&
> > +	head2_parent=$(git cat-file -p HEAD | grep parent | cut -f
> > 2 -d" ") &&
> 
> Do not use "cat-file -p" that is for human consumption in scripts,
> unless you are testing how the format for human consumption should
> look like (we may later add more pretty-print to them), which is not
> the case here.
> 
> Also be careful and pay attention to the end of the header; you do
> not want to pick up a random "parent" string in the middle of a log
> message.
> 
>     head2_parent=$(git cat-file commit HEAD | sed -n -e
> "s/^parent //p" -e "/^$/q")
> 
> would be much better.
> 
> > +	head2_parent_7=$(echo $head2_parent | cut -c1-7) &&
> > +	tree2=$(git cat-file -p HEAD | grep tree | cut -f 2 -d" ")
> > &&
> 
> Likewise.
> 
> > +	tree2_7=$(echo $tree2 | cut -c1-7)
> 
> Likewise.
but is there "git something" to return abbreviated tree hash except
"pretty formats" that is implicitly tested here?

^ permalink raw reply

* [PATCH v2] add: warn when -u or -A is used without filepattern
From: Matthieu Moy @ 2013-01-25 10:49 UTC (permalink / raw)
  To: git, gitster
  Cc: Jonathan Nieder, Robin Rosenberg, Piotr Krukowiecki,
	Eric James Michael Ritz, Tomas Carnecky, Matthieu Moy
In-Reply-To: <vpq1uddoedj.fsf@grenoble-inp.fr>

Most git commands that can be used with our without a filepattern are
tree-wide by default, the filepattern being used to restrict their scope.
A few exceptions are: 'git grep', 'git clean', 'git add -u' and 'git add -A'.

The inconsistancy of 'git add -u' and 'git add -A' are particularly
problematic since other 'git add' subcommands (namely 'git add -p' and
'git add -e') are tree-wide by default.

Flipping the default now is unacceptable, so this patch starts training
users to type explicitely 'git add -u|-A :/' or 'git add -u|-A .', to prepare
for the next steps:

* forbid 'git add -u|-A' without filepattern (like 'git add' without
  option)

* much later, maybe, re-allow 'git add -u|-A' without filepattern, with a
  tree-wide scope.

A nice side effect of this patch is that it makes the :/ special
filepattern easier to discover for users.

When the command is called from the root of the tree, there is no
ambiguity and no need to change the behavior, hence no need to warn.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Changes since v1:

* Do not warn from the root of the tree.

* Say explicitely "Git 2.0" to announce the change.

(plus fix a C99 style issue)

 Documentation/git-add.txt |  7 ++++---
 builtin/add.c             | 36 +++++++++++++++++++++++++++++++++++-
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index fd9e36b..5333559 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -107,9 +107,10 @@ apply to the index. See EDITING PATCHES below.
 	from the index if the corresponding files in the working tree
 	have been removed.
 +
-If no <filepattern> is given, default to "."; in other words,
-update all tracked files in the current directory and its
-subdirectories.
+If no <filepattern> is given, the current version of Git defaults to
+"."; in other words, update all tracked files in the current directory
+and its subdirectories. This default will change in a future version
+of Git, hence the form without <filepattern> should not be used.
 
 -A::
 --all::
diff --git a/builtin/add.c b/builtin/add.c
index e664100..8252d19 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -363,6 +363,33 @@ static int add_files(struct dir_struct *dir, int flags)
 	return exit_status;
 }
 
+static void warn_pathless_add(const char *option_name) {
+	/*
+	 * To be consistant with "git add -p" and most Git
+	 * commands, we should default to being tree-wide, but
+	 * this is not the original behavior and can't be
+	 * changed until users trained themselves not to type
+	 * "git add -u" or "git add -A". For now, we warn and
+	 * keep the old behavior. Later, this warning can be
+	 * turned into a die(...), and eventually we may
+	 * reallow the command with a new behavior.
+	 */
+	warning(_("The behavior of 'git add %s' with no path argument from a subdirectory of the\n"
+		  "tree will change in Git 2.0 and shouldn't be used anymore.\n"
+		  "To add content for the whole tree, run:\n"
+		  "\n"
+		  "  git add %s :/\n"
+		  "\n"
+		  "To restrict the command to the current directory, run:\n"
+		  "\n"
+		  "  git add %s .\n"
+		  "\n"
+		  "With the current Git version, the command is restricted to the current directory."),
+		option_name,
+		option_name,
+		option_name);
+}
+
 int cmd_add(int argc, const char **argv, const char *prefix)
 {
 	int exit_status = 0;
@@ -373,6 +400,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	int add_new_files;
 	int require_pathspec;
 	char *seen = NULL;
+	const char *option_with_implicit_dot = NULL;
 
 	git_config(add_config, NULL);
 
@@ -392,8 +420,14 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		die(_("-A and -u are mutually incompatible"));
 	if (!show_only && ignore_missing)
 		die(_("Option --ignore-missing can only be used together with --dry-run"));
-	if ((addremove || take_worktree_changes) && !argc) {
+	if (addremove)
+		option_with_implicit_dot = "--all";
+	if (take_worktree_changes)
+		option_with_implicit_dot = "--update";
+	if (option_with_implicit_dot && !argc) {
 		static const char *here[2] = { ".", NULL };
+		if (prefix)
+			warn_pathless_add(option_with_implicit_dot);
 		argc = 1;
 		argv = here;
 	}
-- 
1.8.0.1.527.gd366564.dirty

^ permalink raw reply related

* Re: [PATCH 4/4] git-difftool: use git-mergetool--lib for "--tool-help"
From: John Keeping @ 2013-01-25 10:47 UTC (permalink / raw)
  To: David Aguilar; +Cc: git
In-Reply-To: <CAJDDKr5Xd4-e6VO-iO=EmwOcg0RimJHyO4ey-dFk6aENt+qZBQ@mail.gmail.com>

On Fri, Jan 25, 2013 at 01:55:03AM -0800, David Aguilar wrote:
> list_merge_tool_candidates() has a bunch of other special cases
> for $EDITOR, $DISPLAY, $GNOME-something and such so I think
> we should keep using it only for the guess_merge_tool() path.
> 
> I honestly want to remove list_merge_tool_candidates every
> time I read it, but I recognize that it does serve a purpose
> for users who have not configured anything.

Actually, I'm not sure it does.  I asked one of my colleagues whether
he used git-mergetool the other day and he said no because he couldn't
understand the OS X FileMerge tool and was happier to edit things
manually in vim.  I don't think he'd realised that he could configure a
different mergetool.

Perhaps we're trying to be too clever by guessing what the user wants
and should instead exit with a message saying:

   You have not configured a merge tool to use.  Please select one of
   the following tools and configure it using:

        git config merge.tool <tool>

    These tools are availalble on your system:
        ...

    These tools are supported but unavailable:
        ...

This may be too much of a regression for current users though.


John

^ permalink raw reply

* Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim
From: David Aguilar @ 2013-01-25 10:40 UTC (permalink / raw)
  To: John Keeping; +Cc: Junio C Hamano, git
In-Reply-To: <20130125103845.GX7498@serenity.lan>

On Fri, Jan 25, 2013 at 2:38 AM, John Keeping <john@keeping.me.uk> wrote:
> On Fri, Jan 25, 2013 at 01:43:53AM -0800, David Aguilar wrote:
>> "git difftool --tool-help" and "git mergetool --tool-help" incorreclty
>> list "vim" as being an unavailable tool.  This is because they attempt
>> to find a tool named according to the mergetool scriptlet instead of the Git-
>> recognized tool name.
>
> Actually, after my patches both git-difftool and git-mergetool get this
> right since list_merge_tool_candidates lists vimdiff and gvimdiff.
>
>> vimdiff, vimdiff2, gvimdiff, and gvimdiff2 are all provided by the "vim"
>> scriptlet.  This required git-mergetool--lib to special-case it when
>> setting up the tool.
>>
>> Remove the exception for "vim" and allow the scriptlets to be found
>> naturally by using symlinks to a single "vimdiff" scriptlet.
>
> I wonder if it would be better to make these single-line scripts instead
> of symlinks:
>
>     . "$MERGE_TOOLS_DIR"/vimdiff
>
> where we make git-mergetool--lib.sh export:
>
>     MERGE_TOOLS_DIR=$(git --exec-path)/mergetools

That sounds like the way to go.
-- 
David

^ permalink raw reply

* Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim
From: John Keeping @ 2013-01-25 10:38 UTC (permalink / raw)
  To: David Aguilar; +Cc: Junio C Hamano, git
In-Reply-To: <1359107034-14606-7-git-send-email-davvid@gmail.com>

On Fri, Jan 25, 2013 at 01:43:53AM -0800, David Aguilar wrote:
> "git difftool --tool-help" and "git mergetool --tool-help" incorreclty
> list "vim" as being an unavailable tool.  This is because they attempt
> to find a tool named according to the mergetool scriptlet instead of the Git-
> recognized tool name.

Actually, after my patches both git-difftool and git-mergetool get this
right since list_merge_tool_candidates lists vimdiff and gvimdiff.

> vimdiff, vimdiff2, gvimdiff, and gvimdiff2 are all provided by the "vim"
> scriptlet.  This required git-mergetool--lib to special-case it when
> setting up the tool.
> 
> Remove the exception for "vim" and allow the scriptlets to be found
> naturally by using symlinks to a single "vimdiff" scriptlet.

I wonder if it would be better to make these single-line scripts instead
of symlinks:

    . "$MERGE_TOOLS_DIR"/vimdiff

where we make git-mergetool--lib.sh export:

    MERGE_TOOLS_DIR=$(git --exec-path)/mergetools


John

^ permalink raw reply

* git merge error question: The following untracked working tree files would be overwritten by merge
From: Carsten Fuchs @ 2013-01-25 10:37 UTC (permalink / raw)
  To: git

Hi all,

in my repo, I'm doing this:

 > $ git status
 > # On branch master
 > # Your branch is behind 'origin/master' by 2 commits, and can be fast-forwarded.
 > #
 > # Untracked files:
 > #   (use "git add <file>..." to include in what will be committed)
 > #
 > #       obsolete/
 > nothing added to commit but untracked files present (use "git add" to track)
 >
 > $ git merge origin/master --ff-only
 > Updating f419d57..2da6052
 > error: The following untracked working tree files would be overwritten by merge:
 >         obsolete/e107/Readme.txt
 >         obsolete/e107/article.php
 >         obsolete/e107/backend.php
 >         [...]


That is, the local repository has the untracked directory "obsolete", which was added 
upstream as well, and now I try to reconcile.

I seem to understand the problem stated in the error message, and the solution seems to 
be simple as well: renaming the obsolete/ directory is enough.

But why does Git find a problem here at all?

Compare with what Subversion did in an analogous case: When I ran "svn update" and the 
update brought new files for which there already was an untracked copy in the working 
directory, Subversion:
     - started to consider the file as tracked,
     - but left the file in the working-copy alone.

As a result, a subsequent "svn status" might
     a) no longer show the file at all, if the foreign copy in the working directory 
happened to be the same as the one brought by the "svn update", or
     b) flag the file as modified, if different from the one that "svn update" would 
have created in its place.

So my real question is, why does Git not do something analogous?
(Afaics, update the HEAD, update the Index, but leave the working-copy edition alone?)

I searched for this beforehand, and most advice involves either stashing, or with "git 
reset --hard" the loss of the untracked files.

Sorry if this is a stupid question -- I still consider myself a Git learner.

Best regards,
Carsten

^ permalink raw reply

* Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim
From: Sebastian Schuberth @ 2013-01-25 10:34 UTC (permalink / raw)
  To: David Aguilar; +Cc: Junio C Hamano, git, John Keeping
In-Reply-To: <CAJDDKr4yb1-C9W1ZnKxHUk7WKpq-EN_YiR6diHCm5DVtQWzeCQ@mail.gmail.com>

On Fri, Jan 25, 2013 at 11:28 AM, David Aguilar <davvid@gmail.com> wrote:

> I thought Git did something sensible there like create a normal file?

It does not. Also see my answer over here:
http://stackoverflow.com/questions/11412028/git-not-storing-symlink-as-a-symlink-on-windows/11412341#11412341

> If not then I was thinking we could add a provides_tools() function that
> each scriptlet could supply.

Or maybe simply introduce scriptlets that do nothing but include /
inline another scriptlet?

-- 
Sebastian Schuberth

^ permalink raw reply

* Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim
From: David Aguilar @ 2013-01-25 10:28 UTC (permalink / raw)
  To: Sebastian Schuberth; +Cc: Junio C Hamano, git, John Keeping
In-Reply-To: <51025D1C.2030307@gmail.com>

On Fri, Jan 25, 2013 at 2:23 AM, Sebastian Schuberth
<sschuberth@gmail.com> wrote:
> On 2013/01/25 10:43 , David Aguilar wrote:
>
>> Remove the exception for "vim" and allow the scriptlets to be found
>> naturally by using symlinks to a single "vimdiff" scriptlet.  This
>
>
> I guess that won't work on platforms where Git does not support symlinks,
> then, like Windows. But Windows has (g)vimdiff, so loosing these on that
> platform would not be so nice.

I thought Git did something sensible there like create a normal file?

If not then I was thinking we could add a provides_tools() function that
each scriptlet could supply.
-- 
David

^ permalink raw reply

* Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim
From: Sebastian Schuberth @ 2013-01-25 10:23 UTC (permalink / raw)
  To: David Aguilar; +Cc: Junio C Hamano, git, John Keeping
In-Reply-To: <1359107034-14606-7-git-send-email-davvid@gmail.com>

On 2013/01/25 10:43 , David Aguilar wrote:

> Remove the exception for "vim" and allow the scriptlets to be found
> naturally by using symlinks to a single "vimdiff" scriptlet.  This

I guess that won't work on platforms where Git does not support 
symlinks, then, like Windows. But Windows has (g)vimdiff, so loosing 
these on that platform would not be so nice.

-- 
Sebastian Schuberth

^ permalink raw reply

* Re: [PATCH] mergetools: Enhance tortoisemerge to work with
From: David Aguilar @ 2013-01-25 10:09 UTC (permalink / raw)
  To: Sven Strickroth; +Cc: git, Junio C Hamano, Sebastian Schuberth, Jeff King
In-Reply-To: <51024B02.9020400@tu-clausthal.de>

On Fri, Jan 25, 2013 at 1:06 AM, Sven Strickroth
<sven.strickroth@tu-clausthal.de> wrote:
> TortoiseGitMerge and filenames with spaces
>
> - The TortoiseGit team renamed TortoiseMerge.exe to TortoiseGitMerge.exe
>   (starting with 1.8.0) in order to make clear that this one has special
>   support for git, (uses spaces as cli parameter key-value separators)
>   and prevent confusion with the TortoiseSVN TortoiseMerge version.
> - The tortoisemerge mergetool does not work with filenames which have
>   a space in it. Fixing this required changes in git and also in
>   TortoiseGitMerge; see https://github.com/msysgit/msysgit/issues/57.
>
> Signed-off-by: Sven Strickroth <email@cs-ware.de>
> Reported-by: Sebastian Schuberth <sschuberth@gmail.com>
> ---
>  mergetools/tortoisemerge | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge
> index ed7db49..9890737 100644
> --- a/mergetools/tortoisemerge
> +++ b/mergetools/tortoisemerge
> @@ -6,12 +6,28 @@ merge_cmd () {
>         if $base_present
>         then
>                 touch "$BACKUP"
> -               "$merge_tool_path" \
> -                       -base:"$BASE" -mine:"$LOCAL" \
> -                       -theirs:"$REMOTE" -merged:"$MERGED"
> +               if test "$merge_tool_path" == "tortoisegitmerge"

I like the approach this is taking.  Thank you.
I have one small note:

I think this should use "=" instead of "==" here.

It might also make sense to wrap a basename call around it
so that users can set their own mergetool.tortoisemerge.path

basename="$(basename "$merge_tool_path" .exe)"
if test "$basename" = "tortoisegitmerge"
...


> +               then
> +                       "$merge_tool_path" \
> +                               -base "$BASE" -mine "$LOCAL" \
> +                               -theirs "$REMOTE" -merged "$MERGED"
> +               else
> +                       "$merge_tool_path" \
> +                               -base:"$BASE" -mine:"$LOCAL" \
> +                               -theirs:"$REMOTE" -merged:"$MERGED"
> +               fi
>                 check_unchanged
>         else
> -               echo "TortoiseMerge cannot be used without a base" 1>&2
> +               echo "$merge_tool_path cannot be used without a base" 1>&2
>                 return 1
>         fi
>  }
> +
> +translate_merge_tool_path() {
> +       if type tortoisegitmerge >/dev/null 2>/dev/null
> +       then
> +               echo tortoisegitmerge
> +       else
> +               echo tortoisemerge
> +       fi
> +}
> --
> Best regards,
>  Sven Strickroth
>  PGP key id F5A9D4C4 @ any key-server
-- 
David

^ permalink raw reply

* Re: [PATCH 4/4] git-difftool: use git-mergetool--lib for "--tool-help"
From: David Aguilar @ 2013-01-25  9:55 UTC (permalink / raw)
  To: John Keeping; +Cc: git
In-Reply-To: <20130125091918.GV7498@serenity.lan>

On Fri, Jan 25, 2013 at 1:19 AM, John Keeping <john@keeping.me.uk> wrote:
> On Thu, Jan 24, 2013 at 09:29:58PM -0800, David Aguilar wrote:
>> On Thu, Jan 24, 2013 at 11:55 AM, John Keeping <john@keeping.me.uk> wrote:
>> > The "--tool-help" option to git-difftool currently displays incorrect
>> > output since it uses the names of the files in
>> > "$GIT_EXEC_PATH/mergetools/" rather than the list of command names in
>> > git-mergetool--lib.
>> >
>> > Fix this by simply delegating the "--tool-help" argument to the
>> > show_tool_help function in git-mergetool--lib.
>>
>> Very nice.
>>
>> One thought I had was that the unified show_tool_help should
>> probably check TOOL_MODE=diff and skip over the
>> !can_diff entries.
>>
>> The current output of "git difftool --tool-help" before your
>> patches has the problem that it will list tools such as
>> "tortoisemerge" as "valid but not available" because it
>> does not differentiate between missing and !can_diff.
>
> list_merge_tool_candidates does this for us, so it should Just Work
> since we use that to generate the list of tools that we loop over.

Yup, kind of.  I looked more closely and found a lot of
special-cases around vim which I've eliminated in the series
I just sent (which includes your patches as its base).

list_merge_tool_candidates() has a bunch of other special cases
for $EDITOR, $DISPLAY, $GNOME-something and such so I think
we should keep using it only for the guess_merge_tool() path.

I honestly want to remove list_merge_tool_candidates every
time I read it, but I recognize that it does serve a purpose
for users who have not configured anything.

In order to be useful for the documentation I think the
code could be refactored a tiny bit more beyond what I've
sent so far.  The gathering of available tools can be split
off from the reporting, and then show_tool_help() can use
the gatherer.  With what I sent, though, there's at least
a 1:1 correspondance between the name of the scriptlets
and the names accepted by --tool=<tool>, which is the first
step towards doing that.

I have to check out for now but I'll keep poking at this
when the weekend rolls around.

cheers,
-- 
David

^ permalink raw reply

* Re: [PATCH] mergetools: Add tortoisegitmerge helper
From: John Keeping @ 2013-01-25  9:48 UTC (permalink / raw)
  To: David Aguilar
  Cc: Junio C Hamano, Sven Strickroth, git, Sebastian Schuberth,
	Jeff King
In-Reply-To: <CAJDDKr4oerSq16rYt2iKNtQNK79L+jOiKROhEW_yiBPKjkVhuQ@mail.gmail.com>

On Thu, Jan 24, 2013 at 11:54:25PM -0800, David Aguilar wrote:
> On Thu, Jan 24, 2013 at 11:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Is there a way for me to programatically tell what merge.tool and
> > diff.tool could be enabled for a particular source checkout of Git
> > regardless of what platform am I on (that is, even though I won't
> > touch Windows, I want to see 'tortoise' appear in the output of such
> > a procedure)?  We could generate a small text file from the Makefile
> > in Documentation and include it when building the manual pages if
> > such a procedure is available.
> 
> That's a good idea.
> Here's one way... (typed into gmail, so probably broken)
>
> LF='
> '
> mergetools=
> difftools=
> scriptlets="$(git --exec-path)"/mergetools
> 
> for script in "$scriptlets"/*
> do
>     tool="$(basename "$script")"
>     if test "$tool" = "defaults"
>     then
>         continue
>     fi
>     . "$scriptlets"/defaults
>     can_diff && difftools="$difftools$tool$LF"
>     can_merge && mergetools="$mergetools$tool$LF"
> done

I don't think this will work since the names of the valid tools are not
necessarily the same as the names of the scriptlets - this is the exact
issue that prompted my patches to git-difftool yesterday.

The best option I can see given what's currently available is something
like this:

-- >8 --

sed -n -e '/^list_merge_tool_candidates/,/^}/ {
        /tools=/ {
                s/.*tools=//
                s/"//g
                s/\$tools//
                s/ /\n/g
                p
        }
}' git-mergetool--lib.sh |sort |uniq |while read -r tool
do
	test -z "$tool" && continue
	( . git-mergetool--lib && setup_tool $tool
        	# Use can_diff and can_merge here.
	)
done

-- 8< --


John

^ permalink raw reply

* [PATCH 2/7] git-mergetool: remove redundant assignment
From: David Aguilar @ 2013-01-25  9:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, John Keeping
In-Reply-To: <1359107034-14606-1-git-send-email-davvid@gmail.com>

From: John Keeping <john@keeping.me.uk>

TOOL_MODE is set at the top of git-mergetool.sh so there is no need to
set it again in show_tool_help.  Removing this lets us re-use
show_tool_help in git-difftool.

Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-mergetool--lib.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 89a857f..1748315 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -175,7 +175,6 @@ list_merge_tool_candidates () {
 }
 
 show_tool_help () {
-	TOOL_MODE=merge
 	list_merge_tool_candidates
 	unavailable= available= LF='
 '
-- 
1.8.1.1.367.g22b1720.dirty

^ permalink raw reply related

* [PATCH 1/7] git-mergetool: move show_tool_help to mergetool--lib
From: David Aguilar @ 2013-01-25  9:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, John Keeping
In-Reply-To: <1359107034-14606-1-git-send-email-davvid@gmail.com>

From: John Keeping <john@keeping.me.uk>

This is the first step in unifying "git difftool --tool-help" and
"git mergetool --tool-help".

Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-mergetool--lib.sh | 37 +++++++++++++++++++++++++++++++++++++
 git-mergetool.sh      | 37 -------------------------------------
 2 files changed, 37 insertions(+), 37 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index f013a03..89a857f 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -174,6 +174,43 @@ list_merge_tool_candidates () {
 	esac
 }
 
+show_tool_help () {
+	TOOL_MODE=merge
+	list_merge_tool_candidates
+	unavailable= available= LF='
+'
+	for i in $tools
+	do
+		merge_tool_path=$(translate_merge_tool_path "$i")
+		if type "$merge_tool_path" >/dev/null 2>&1
+		then
+			available="$available$i$LF"
+		else
+			unavailable="$unavailable$i$LF"
+		fi
+	done
+	if test -n "$available"
+	then
+		echo "'git mergetool --tool=<tool>' may be set to one of the following:"
+		echo "$available" | sort | sed -e 's/^/	/'
+	else
+		echo "No suitable tool for 'git mergetool --tool=<tool>' found."
+	fi
+	if test -n "$unavailable"
+	then
+		echo
+		echo 'The following tools are valid, but not currently available:'
+		echo "$unavailable" | sort | sed -e 's/^/	/'
+	fi
+	if test -n "$unavailable$available"
+	then
+		echo
+		echo "Some of the tools listed above only work in a windowed"
+		echo "environment. If run in a terminal-only session, they will fail."
+	fi
+	exit 0
+}
+
 guess_merge_tool () {
 	list_merge_tool_candidates
 	echo >&2 "merge tool candidates: $tools"
diff --git a/git-mergetool.sh b/git-mergetool.sh
index c50e18a..c0ee9aa 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -315,43 +315,6 @@ merge_file () {
 	return 0
 }
 
-show_tool_help () {
-	TOOL_MODE=merge
-	list_merge_tool_candidates
-	unavailable= available= LF='
-'
-	for i in $tools
-	do
-		merge_tool_path=$(translate_merge_tool_path "$i")
-		if type "$merge_tool_path" >/dev/null 2>&1
-		then
-			available="$available$i$LF"
-		else
-			unavailable="$unavailable$i$LF"
-		fi
-	done
-	if test -n "$available"
-	then
-		echo "'git mergetool --tool=<tool>' may be set to one of the following:"
-		echo "$available" | sort | sed -e 's/^/	/'
-	else
-		echo "No suitable tool for 'git mergetool --tool=<tool>' found."
-	fi
-	if test -n "$unavailable"
-	then
-		echo
-		echo 'The following tools are valid, but not currently available:'
-		echo "$unavailable" | sort | sed -e 's/^/	/'
-	fi
-	if test -n "$unavailable$available"
-	then
-		echo
-		echo "Some of the tools listed above only work in a windowed"
-		echo "environment. If run in a terminal-only session, they will fail."
-	fi
-	exit 0
-}
-
 prompt=$(git config --bool mergetool.prompt || echo true)
 
 while test $# != 0
-- 
1.8.1.1.367.g22b1720.dirty

^ permalink raw reply related

* [PATCH 4/7] git-difftool: use git-mergetool--lib for "--tool-help"
From: David Aguilar @ 2013-01-25  9:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, John Keeping
In-Reply-To: <1359107034-14606-1-git-send-email-davvid@gmail.com>

From: John Keeping <john@keeping.me.uk>

The "--tool-help" option to git-difftool currently displays incorrect
output since it uses the names of the files in
"$GIT_EXEC_PATH/mergetools/" rather than the list of command names in
git-mergetool--lib.

Fix this by simply delegating the "--tool-help" argument to the
show_tool_help function in git-mergetool--lib.

Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-difftool.perl | 55 +++++++------------------------------------------------
 1 file changed, 7 insertions(+), 48 deletions(-)

diff --git a/git-difftool.perl b/git-difftool.perl
index edd0493..0a90de4 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -59,57 +59,16 @@ sub find_worktree
 	return $worktree;
 }
 
-sub filter_tool_scripts
-{
-	my ($tools) = @_;
-	if (-d $_) {
-		if ($_ ne ".") {
-			# Ignore files in subdirectories
-			$File::Find::prune = 1;
-		}
-	} else {
-		if ((-f $_) && ($_ ne "defaults")) {
-			push(@$tools, $_);
-		}
-	}
-}
-
 sub print_tool_help
 {
-	my ($cmd, @found, @notfound, @tools);
-	my $gitpath = Git::exec_path();
-
-	find(sub { filter_tool_scripts(\@tools) }, "$gitpath/mergetools");
-
-	foreach my $tool (@tools) {
-		$cmd  = "TOOL_MODE=diff";
-		$cmd .= ' && . "$(git --exec-path)/git-mergetool--lib"';
-		$cmd .= " && get_merge_tool_path $tool >/dev/null 2>&1";
-		$cmd .= " && can_diff >/dev/null 2>&1";
-		if (system('sh', '-c', $cmd) == 0) {
-			push(@found, $tool);
-		} else {
-			push(@notfound, $tool);
-		}
-	}
-
-	print << 'EOF';
-'git difftool --tool=<tool>' may be set to one of the following:
-EOF
-	print "\t$_\n" for (sort(@found));
+	my $cmd = 'TOOL_MODE=diff';
+	$cmd .= ' && . "$(git --exec-path)/git-mergetool--lib"';
+	$cmd .= ' && show_tool_help';
 
-	print << 'EOF';
-
-The following tools are valid, but not currently available:
-EOF
-	print "\t$_\n" for (sort(@notfound));
-
-	print << 'EOF';
-
-NOTE: Some of the tools listed above only work in a windowed
-environment. If run in a terminal-only session, they will fail.
-EOF
-	exit(0);
+	# See the comment at the bottom of file_diff() for the reason behind
+	# using system() followed by exit() instead of exec().
+	my $rc = system('sh', '-c', $cmd);
+	exit($rc | ($rc >> 8));
 }
 
 sub exit_cleanup
-- 
1.8.1.1.367.g22b1720.dirty

^ permalink raw reply related

* [PATCH 0/7] mergetool-lib improvements for --tool-help
From: David Aguilar @ 2013-01-25  9:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, John Keeping

I ran with John's idea and simplified a few more things so
that the --tool-help output shows the correct results.

My final commit is dependent on John's commits but the other two
could be picked up independently since they are general improvements.

This does add a few symlinks to the repo for the sake of simplicity;
I'm not sure if this presents a problem for folks on other platforms.

The replacement of vim with vimdiff and symlinks simplifies things
so that we can later auto-generate the list-of-all-tools for use
by Documentation/.  This was not previously possible due to the
special-case for the vim tools which this series eliminates.

David Aguilar (3):
  mergetools/vim: Remove redundant diff command
  mergetools: Fix difftool/mergetool --tool-help listing for vim
  mergetool--lib: Improve show_tool_help() output

John Keeping (4):
  git-mergetool: move show_tool_help to mergetool--lib
  git-mergetool: remove redundant assignment
  git-mergetool: don't hardcode 'mergetool' in show_tool_help
  git-difftool: use git-mergetool--lib for "--tool-help"

 git-difftool.perl           | 55 +++++----------------------------------
 git-mergetool--lib.sh       | 63 +++++++++++++++++++++++++++++++++++++++------
 git-mergetool.sh            | 37 --------------------------
 mergetools/gvimdiff         |  1 +
 mergetools/gvimdiff2        |  1 +
 mergetools/{vim => vimdiff} | 12 ++-------
 mergetools/vimdiff2         |  1 +
 7 files changed, 67 insertions(+), 103 deletions(-)
 create mode 120000 mergetools/gvimdiff
 create mode 120000 mergetools/gvimdiff2
 rename mergetools/{vim => vimdiff} (68%)
 create mode 120000 mergetools/vimdiff2

-- 
1.8.1.1.367.g22b1720.dirty

^ permalink raw reply

* [PATCH 7/7] mergetool--lib: Improve show_tool_help() output
From: David Aguilar @ 2013-01-25  9:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, John Keeping
In-Reply-To: <1359107034-14606-1-git-send-email-davvid@gmail.com>

Check the can_diff and can_merge functions before deciding whether to
add the tool to the available/unavailable lists.  This makes --tool-help context-
sensitive so that "git mergetool --tool-help" displays merge tools only
and "git difftool --tool-help" displays diff tools only.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-mergetool--lib.sh | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index db8218a..c547c59 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -168,17 +168,33 @@ list_merge_tool_candidates () {
 }
 
 show_tool_help () {
-	list_merge_tool_candidates
 	unavailable= available= LF='
 '
-	for i in $tools
+
+	scriptlets="$(git --exec-path)"/mergetools
+	for i in "$scriptlets"/*
 	do
-		merge_tool_path=$(translate_merge_tool_path "$i")
+		. "$scriptlets"/defaults
+		. "$i"
+
+		tool="$(basename "$i")"
+		if test "$tool" = "defaults"
+		then
+			continue
+		elif merge_mode && ! can_merge
+		then
+			continue
+		elif diff_mode && ! can_diff
+		then
+			continue
+		fi
+
+		merge_tool_path=$(translate_merge_tool_path "$tool")
 		if type "$merge_tool_path" >/dev/null 2>&1
 		then
-			available="$available$i$LF"
+			available="$available$tool$LF"
 		else
-			unavailable="$unavailable$i$LF"
+			unavailable="$unavailable$tool$LF"
 		fi
 	done
 
-- 
1.8.1.1.367.g22b1720.dirty

^ permalink raw reply related

* [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim
From: David Aguilar @ 2013-01-25  9:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, John Keeping
In-Reply-To: <1359107034-14606-1-git-send-email-davvid@gmail.com>

"git difftool --tool-help" and "git mergetool --tool-help" incorreclty
list "vim" as being an unavailable tool.  This is because they attempt
to find a tool named according to the mergetool scriptlet instead of the Git-
recognized tool name.

vimdiff, vimdiff2, gvimdiff, and gvimdiff2 are all provided by the "vim"
scriptlet.  This required git-mergetool--lib to special-case it when
setting up the tool.

Remove the exception for "vim" and allow the scriptlets to be found
naturally by using symlinks to a single "vimdiff" scriptlet.  This
causes the --tool-help option to correctly list vimdiff, vimdiff2,
gvimdiff, and gvimdiff2 in its output.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-mergetool--lib.sh       | 9 +--------
 mergetools/gvimdiff         | 1 +
 mergetools/gvimdiff2        | 1 +
 mergetools/{vim => vimdiff} | 0
 mergetools/vimdiff2         | 1 +
 5 files changed, 4 insertions(+), 8 deletions(-)
 create mode 120000 mergetools/gvimdiff
 create mode 120000 mergetools/gvimdiff2
 rename mergetools/{vim => vimdiff} (100%)
 create mode 120000 mergetools/vimdiff2

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 4c1e129..db8218a 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -44,14 +44,7 @@ valid_tool () {
 }
 
 setup_tool () {
-	case "$1" in
-	vim*|gvim*)
-		tool=vim
-		;;
-	*)
-		tool="$1"
-		;;
-	esac
+	tool="$1"
 	mergetools="$(git --exec-path)/mergetools"
 
 	# Load the default definitions
diff --git a/mergetools/gvimdiff b/mergetools/gvimdiff
new file mode 120000
index 0000000..2a72558
--- /dev/null
+++ b/mergetools/gvimdiff
@@ -0,0 +1 @@
+vimdiff
\ No newline at end of file
diff --git a/mergetools/gvimdiff2 b/mergetools/gvimdiff2
new file mode 120000
index 0000000..2a72558
--- /dev/null
+++ b/mergetools/gvimdiff2
@@ -0,0 +1 @@
+vimdiff
\ No newline at end of file
diff --git a/mergetools/vim b/mergetools/vimdiff
similarity index 100%
rename from mergetools/vim
rename to mergetools/vimdiff
diff --git a/mergetools/vimdiff2 b/mergetools/vimdiff2
new file mode 120000
index 0000000..2a72558
--- /dev/null
+++ b/mergetools/vimdiff2
@@ -0,0 +1 @@
+vimdiff
\ No newline at end of file
-- 
1.8.1.1.367.g22b1720.dirty

^ permalink raw reply related

* [PATCH 5/7] mergetools/vim: Remove redundant diff command
From: David Aguilar @ 2013-01-25  9:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, John Keeping
In-Reply-To: <1359107034-14606-1-git-send-email-davvid@gmail.com>

vimdiff and vimdiff2 differ only by their merge command so remove the
logic in the diff command since it's not actually needed.

Signed-off-by: David Aguilar <davvid@gmail.com>
---
 mergetools/vim | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/mergetools/vim b/mergetools/vim
index 619594a..39d0327 100644
--- a/mergetools/vim
+++ b/mergetools/vim
@@ -1,14 +1,6 @@
 diff_cmd () {
-	case "$1" in
-	gvimdiff|vimdiff)
-		"$merge_tool_path" -R -f -d \
-			-c 'wincmd l' -c 'cd $GIT_PREFIX' "$LOCAL" "$REMOTE"
-		;;
-	gvimdiff2|vimdiff2)
-		"$merge_tool_path" -R -f -d \
-			-c 'wincmd l' -c 'cd $GIT_PREFIX' "$LOCAL" "$REMOTE"
-		;;
-	esac
+	"$merge_tool_path" -R -f -d \
+		-c 'wincmd l' -c 'cd $GIT_PREFIX' "$LOCAL" "$REMOTE"
 }
 
 merge_cmd () {
-- 
1.8.1.1.367.g22b1720.dirty

^ permalink raw reply related

* [PATCH 3/7] git-mergetool: don't hardcode 'mergetool' in show_tool_help
From: David Aguilar @ 2013-01-25  9:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, John Keeping
In-Reply-To: <1359107034-14606-1-git-send-email-davvid@gmail.com>

From: John Keeping <john@keeping.me.uk>

When using show_tool_help from git-difftool we will want it to print
"git difftool" not "git mergetool" so use "git ${TOOL_MODE}tool".

Signed-off-by: John Keeping <john@keeping.me.uk>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
 git-mergetool--lib.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 1748315..4c1e129 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -188,12 +188,14 @@ show_tool_help () {
 			unavailable="$unavailable$i$LF"
 		fi
 	done
+
+	cmd_name=${TOOL_MODE}tool
 	if test -n "$available"
 	then
-		echo "'git mergetool --tool=<tool>' may be set to one of the following:"
+		echo "'git $cmd_name --tool=<tool>' may be set to one of the following:"
 		echo "$available" | sort | sed -e 's/^/	/'
 	else
-		echo "No suitable tool for 'git mergetool --tool=<tool>' found."
+		echo "No suitable tool for 'git $cmd_name --tool=<tool>' found."
 	fi
 	if test -n "$unavailable"
 	then
-- 
1.8.1.1.367.g22b1720.dirty

^ permalink raw reply related

* Re: [PATCH 4/4] git-difftool: use git-mergetool--lib for "--tool-help"
From: John Keeping @ 2013-01-25  9:19 UTC (permalink / raw)
  To: David Aguilar; +Cc: git
In-Reply-To: <CAJDDKr4ZpQr029FW0v8LzwvhXZYmvAONbbZNuOq_E=Q1UzufvA@mail.gmail.com>

On Thu, Jan 24, 2013 at 09:29:58PM -0800, David Aguilar wrote:
> On Thu, Jan 24, 2013 at 11:55 AM, John Keeping <john@keeping.me.uk> wrote:
> > The "--tool-help" option to git-difftool currently displays incorrect
> > output since it uses the names of the files in
> > "$GIT_EXEC_PATH/mergetools/" rather than the list of command names in
> > git-mergetool--lib.
> >
> > Fix this by simply delegating the "--tool-help" argument to the
> > show_tool_help function in git-mergetool--lib.
> 
> Very nice.
> 
> One thought I had was that the unified show_tool_help should
> probably check TOOL_MODE=diff and skip over the
> !can_diff entries.
> 
> The current output of "git difftool --tool-help" before your
> patches has the problem that it will list tools such as
> "tortoisemerge" as "valid but not available" because it
> does not differentiate between missing and !can_diff.

list_merge_tool_candidates does this for us, so it should Just Work
since we use that to generate the list of tools that we loop over.


John

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox