Git development
 help / color / mirror / Atom feed
* Separately repository access in GitWeb
From: Андрей Баранов @ 2013-01-25  8:10 UTC (permalink / raw)
  To: git

good day.

I`m trying make separately repository access in GitWeb by NGINX

separation of access based on URL strings, namely, the presence of the
query strings:
'?p=repo.git'
with a regular expression:
"^.*p=(.*?)(\.git|;|&|=|\s).*$"

I am wondering how much it is correct to protect against unauthorized access.

Thanks in advance :)

complete example of a configuration file:

server {
        listen 80;

root /home/git/gitweb;

        access_log /var/log/nginx/gitweb.access_log main;
        error_log /var/log/nginx/gitweb.error_log info;

        index gitweb.cgi;
gzip off;

    location ~* \.(jpg|txt|jpeg|gif|png|ico|css|zip|js|swf)$ {
access_log        off;
        expires 1d;
    }

    location = / {
set $htpasswd "opened@";
if ($args ~* "^.*p=(.*?)(\.git|;|&|=|\s).*$") {
set $htpasswd /home/git/.gitolite/conf/$1_htpasswd;
        }
        if (-f $htpasswd) {
                rewrite ^.*$  /closed last;
        }

rewrite ^.*$ /guest last;
    }

    location = /closed {
internal;
access_log /var/log/nginx/gitweb-closed.access_log main;
auth_basic "Unauthorized";
        auth_basic_user_file $htpasswd;
        include fastcgi_params;
        fastcgi_param  SCRIPT_NAME gitweb.cgi;
        fastcgi_param SCRIPT_FILENAME /home/git/gitweb/gitweb.cgi;
        fastcgi_pass unix:/var/run/fcgiwrap.socket;
    }

    location = /guest {
internal;
access_log /var/log/nginx/gitweb-guest.access_log main;
        include fastcgi_params;
fastcgi_param  SCRIPT_NAME gitweb.cgi;
fastcgi_param SCRIPT_FILENAME /home/git/gitweb/gitweb.cgi;
        fastcgi_pass unix:/var/run/fcgiwrap.socket;
    }

    location  / {
        rewrite (.*) / permanent;
    }

}

^ permalink raw reply

* [PATCH] branch: reject -D/-d without branch name
From: Nguyễn Thái Ngọc Duy @ 2013-01-25  8:26 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy


Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/branch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 873f624..1d3e842 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -837,7 +837,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		colopts = 0;
 	}
 
-	if (delete)
+	if (delete && argc)
 		return delete_branches(argc, argv, delete > 1, kinds, quiet);
 	else if (list) {
 		int ret = print_ref_list(kinds, detached, verbose, abbrev,
-- 
1.8.1.1.380.g782aa97

^ permalink raw reply related

* Re: [PATCH 1/3] mergetool--lib: fix startup options for gvimdiff tool
From: Alexey Shumkin @ 2013-01-25  8:44 UTC (permalink / raw)
  To: David Aguilar; +Cc: git, Pat Thoyts, Junio C Hamano
In-Reply-To: <CAJDDKr4Zi-pVVtX4LxRv9K7ocjdpLS_5NH5P_wrx0+ZRSwmfFA@mail.gmail.com>

Maybe, some time ;)
Actually, I'm not TCL-programmer. With one of these patches I just have
solved one my problem (to run tortoisemerge with git-gui) when I
was showing to my collegue how to work with Git, and on the side I
fixed another two bugs. So, I decided to sumbit these patches, to avoid
applying them every time after each Git update as I did last 1.5 years
with other patches which still are not submitted, because I'm too lazy
to follow Git development workflow in my free time )

> On Wed, Jan 23, 2013 at 11:16 PM, Alexey Shumkin
> <alex.crezoff@gmail.com> wrote:
> > Options are taken from <Git source>/mergetools/vim
> >
> > Signed-off-by: Alexey Shumkin <Alex.Crezoff@gmail.com>
> > ---
> >  git-gui/lib/mergetool.tcl | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> A better long-term solution might be to teach git gui to use "git
> difftool".
> 
> Would it be better to teach git-gui (and gitk) about
> mergetool/difftool? That would allow us to possibly eliminate this
> duplication.
> 
> We did start towards that path when difftool learned the --extcmd
> option (for use by gitk) but I have not followed through.
> 
> What do you think about trying that approach?
> 
> 
> > diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl
> > index 3c8e73b..4fc1cab 100644
> > --- a/git-gui/lib/mergetool.tcl
> > +++ b/git-gui/lib/mergetool.tcl
> > @@ -211,7 +211,13 @@ proc merge_resolve_tool2 {} {
> >                 }
> >         }
> >         gvimdiff {
> > -               set cmdline [list "$merge_tool_path" -f "$LOCAL"
> > "$MERGED" "$REMOTE"]
> > +               if {$base_stage ne {}} {
> > +                       set cmdline [list "$merge_tool_path" -f -d
> > -c "wincmd J" \
> > +                               "$MERGED" "$LOCAL" "$BASE"
> > "$REMOTE"]
> > +               } else {
> > +                       set cmdline [list "$merge_tool_path" -f -d
> > -c "wincmd l" \
> > +                               "$LOCAL" "$MERGED" "$REMOTE"]
> > +               }
> >         }
> >         kdiff3 {
> >                 if {$base_stage ne {}} {
> > --
> > 1.8.1.1.10.g9255f3f
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe git" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 

^ permalink raw reply

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

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> diff --git a/builtin/branch.c b/builtin/branch.c
> index 873f624..1d3e842 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -837,7 +837,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>  		colopts = 0;
>  	}
>  
> -	if (delete)
> +	if (delete && argc)
>  		return delete_branches(argc, argv, delete > 1, kinds, quiet);
>  	else if (list) {
>  		int ret = print_ref_list(kinds, detached, verbose, abbrev,

Shouldn't this error out with a clean error message ("branch name
expected" or so)?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: [PATCH] branch: reject -D/-d without branch name
From: Duy Nguyen @ 2013-01-25  8:56 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git, Junio C Hamano
In-Reply-To: <vpqmwvxhcra.fsf@grenoble-inp.fr>

On Fri, Jan 25, 2013 at 3:45 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> diff --git a/builtin/branch.c b/builtin/branch.c
>> index 873f624..1d3e842 100644
>> --- a/builtin/branch.c
>> +++ b/builtin/branch.c
>> @@ -837,7 +837,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
>>               colopts = 0;
>>       }
>>
>> -     if (delete)
>> +     if (delete && argc)
>>               return delete_branches(argc, argv, delete > 1, kinds, quiet);
>>       else if (list) {
>>               int ret = print_ref_list(kinds, detached, verbose, abbrev,
>
> Shouldn't this error out with a clean error message ("branch name
> expected" or so)?

Yeah. But on the other hand, this command seems to prefer to print
help usage when incorrect number of arguments is given (checkout
blocks "if (edit_description)" and "if (rename)" in cmd_branch).
Should those be fixed too?
-- 
Duy

^ permalink raw reply

* Fwd: Separately repository access in GitWeb
From: Андрей Баранов @ 2013-01-25  9:00 UTC (permalink / raw)
  To: git
In-Reply-To: <CAJjU7bS-rRr-UuEA_xcqtTY4+tWKshL+Rvey85n70KacSnTorQ@mail.gmail.com>

good day.

I`m trying make separately repository access in GitWeb by NGINX

separation of access based on URL strings, namely, the presence of the
query strings:
'?p=repo.git'
with a regular expression:
"^.*p=(.*?)(\.git|;|&|=|\s).*$"

I am wondering how much it is correct to protect against unauthorized access.

Thanks in advance :)

complete example of a configuration file:

server {
        listen 80;

root /home/git/gitweb;

        access_log /var/log/nginx/gitweb.access_log main;
        error_log /var/log/nginx/gitweb.error_log info;

        index gitweb.cgi;
gzip off;

    location ~* \.(jpg|txt|jpeg|gif|png|ico|css|zip|js|swf)$ {
access_log        off;
        expires 1d;
    }

    location = / {
set $htpasswd "opened@";
if ($args ~* "^.*p=(.*?)(\.git|;|&|=|\s).*$") {
set $htpasswd /home/git/.gitolite/conf/$1_htpasswd;
        }
        if (-f $htpasswd) {
                rewrite ^.*$  /closed last;
        }

rewrite ^.*$ /guest last;
    }

    location = /closed {
internal;
access_log /var/log/nginx/gitweb-closed.access_log main;
auth_basic "Unauthorized";
        auth_basic_user_file $htpasswd;
        include fastcgi_params;
        fastcgi_param  SCRIPT_NAME gitweb.cgi;
        fastcgi_param SCRIPT_FILENAME /home/git/gitweb/gitweb.cgi;
        fastcgi_pass unix:/var/run/fcgiwrap.socket;
    }

    location = /guest {
internal;
access_log /var/log/nginx/gitweb-guest.access_log main;
        include fastcgi_params;
fastcgi_param  SCRIPT_NAME gitweb.cgi;
fastcgi_param SCRIPT_FILENAME /home/git/gitweb/gitweb.cgi;
        fastcgi_pass unix:/var/run/fcgiwrap.socket;
    }

    location  / {
        rewrite (.*) / permanent;
    }

}

^ permalink raw reply

* Re: [PATCH v4 3/4] pretty: Add failing tests: user format ignores i18n.logOutputEncoding setting
From: Alexey Shumkin @ 2013-01-25  9:01 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vip6m9tvj.fsf@alter.siamese.dyndns.org>

> Alexey Shumkin <alex.crezoff@gmail.com> writes:
> 
> > diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
> > index c248509..4db43a4 100755
> > --- a/t/t6006-rev-list-format.sh
> > +++ b/t/t6006-rev-list-format.sh
> > ...
> > @@ -112,12 +133,12 @@ commit $head2
> >  commit $head1
> >  EOF
> >  
> > -test_format raw-body %B <<'EOF'
> > -commit 131a310eb913d107dd3c09a65d1651175898735d
> > -changed foo
> > +test_format failure raw-body %B <<EOF
> > +commit $head2
> > +$changed
> >  
> > -commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
> > -added foo
> > +commit $head1
> > +$added
> >  
> >  EOF
> 
> It may have been easier to follow if you did this "Don't hardcode"
> as a separate preparatory patch, like your first two patches.
Yep, I missed that in my bunch of rebasing ;)

> 
> > @@ -135,16 +156,16 @@ commit $head1
> >  foo
> >  EOF
> >  
> > -cat >commit-msg <<'EOF'
> > +iconv -f utf-8 -t cp1251 > commit-msg <<EOF
> >  Test printing of complex bodies
> >  
> >  This commit message is much longer than the others,
> > -and it will be encoded in iso8859-1. We should therefore
> > -include an iso8859 character: ¡bueno!
> > +and it will be encoded in cp1251. We should therefore
> > +include an cp1251 character: так вот!
> >  EOF
> >  
> >  test_expect_success 'setup complex body' '
> > -	git config i18n.commitencoding iso8859-1 &&
> > +	git config i18n.commitencoding cp1251 &&
> 
> What is going on here?
> 
> Is this an example that shows that i18n.commitencoding works
> correctly with iso8859-1 but not with cp1251?
It show only that I speak and write Russian not Spanish ))
I'll revert back these changes.

> 
> > diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
> > index cf492f4..699c824 100755
> > --- a/t/t7102-reset.sh
> > +++ b/t/t7102-reset.sh
> > ...
> > @@ -192,7 +214,7 @@ test_expect_success \
> >  	'changing files and redo the last commit should succeed' '
> >  	echo "3rd line 2nd file" >>secondfile &&
> >  	git commit -a -C ORIG_HEAD &&
> > -	check_changes 3d3b7be011a58ca0c179ae45d94e6c83c0b0cd0d &&
> > +	check_changes f06f78b8dd468c722952b77569dd0db212442c25 &&
> >  	test "$(git rev-parse ORIG_HEAD)" = \
> >  			$head5
> >  '
> 
> This and remaining hunks to this script shows that it would be
> helped by the same love you gave to other scripts with your first
> two patches before you add the "non-unicode" tests, no?
Sorry, I haven't got you :-[ (it seems, my English is not good enough)
Do you mean "avoid hardcoded SHA-1"?

^ permalink raw reply

* [PATCH] mergetools: Enhance tortoisemerge to work with
From: Sven Strickroth @ 2013-01-25  9:06 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Sebastian Schuberth, davvid, Jeff King
In-Reply-To: <5101B0A5.1020308@tu-clausthal.de>

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"
+		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

^ permalink raw reply related

* Re: [PATCH v4 3/4] pretty: Add failing tests: user format ignores i18n.logOutputEncoding setting
From: Alexey Shumkin @ 2013-01-25  9:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr4la9uqo.fsf@alter.siamese.dyndns.org>

> Alexey Shumkin <alex.crezoff@gmail.com> writes:
> 
> > The following two commands are expected to give the same output to
> > a terminal:
> >
> > 	$ git log --oneline --no-color
> > 	$ git log --pretty=format:'%h %s'
> >
> > However, the former pays attention to i18n.logOutputEncoding
> > configuration, while the latter does not when it format "%s".
> > Log messages written in an encoding i18n.commitEncoding which
> > differs from terminal encoding are shown corrupted with the latter
> > even when i18n.logOutputEncoding and terminal encoding are the same.
> >
> > The same corruption is true for
> > 	$ git diff --submodule=log
> > and
> > 	$ git rev-list --pretty=format:%s HEAD
> > and
> > 	$ git reset --hard
> >
> > Signed-off-by: Alexey Shumkin <Alex.Crezoff@gmail.com>
> > ---
> >  t/t4041-diff-submodule-option.sh | 33 ++++++++-------
> >  t/t4205-log-pretty-formats.sh    | 43 +++++++++++++++----
> >  t/t6006-rev-list-format.sh       | 90
> > ++++++++++++++++++++++++++--------------
> > t/t7102-reset.sh                 | 32 +++++++++++--- 4 files
> > changed, 138 insertions(+), 60 deletions(-)
> >
> > diff --git a/t/t4041-diff-submodule-option.sh
> > b/t/t4041-diff-submodule-option.sh index 32d4a60..e7d6363 100755
> > --- a/t/t4041-diff-submodule-option.sh
> > +++ b/t/t4041-diff-submodule-option.sh
> > @@ -1,6 +1,7 @@
> >  #!/bin/sh
> >  #
> >  # Copyright (c) 2009 Jens Lehmann, based on t7401 by Ping Yin
> > +# Copyright (c) 2011 Alexey Shumkin (+ non-UTF-8 commit encoding
> > tests) #
> >  
> >  test_description='Support for verbose submodule differences in git
> > diff @@ -10,6 +11,7 @@ This test tries to verify the sanity of the
> > --submodule option of git diff. 
> >  . ./test-lib.sh
> >  
> > +added=$(printf
> > "\320\264\320\276\320\261\320\260\320\262\320\273\320\265\320\275")
> 
> Please have an in-code comment before this line to explain what this
> variable is about, e.g.
> 
> 	# String "added" in Russian, encoded in UTF-8, used in
>         # sample commit log messages in add_file() function below.
>         added=$(printf "...")
Ok!

> 
> >  add_file () {
> >  	(
> >  		cd "$1" &&
> > @@ -19,7 +21,8 @@ add_file () {
> >  			echo "$name" >"$name" &&
> >  			git add "$name" &&
> >  			test_tick &&
> > -			git commit -m "Add $name" || exit
> > +			msg_added_cp1251=$(echo "Add $name ($added
> > $name)" | iconv -f utf-8 -t cp1251) &&
> > +			git -c 'i18n.commitEncoding=cp1251' commit
> > -m "$msg_added_cp1251" done >/dev/null &&
> >  		git rev-parse --short --verify HEAD
> >  	)
> 
> Does this patch make the all tests in this script fail for people
> without cp1251 locale installed?  We already have tests that depend
> on 8859-1 and some other locales, and we'd be better off limiting
> such dependency to the minimum.
Hmmm, I'll try to feign something to avoid using cp1251

> 
> Same comment applies to the changes to other test scripts.
> 
> Thanks.

^ permalink raw reply

* Re: [PATCH v4 2/4] t7102 (reset): refactoring: don't hardcode SHA-1 in expected outputs
From: Alexey Shumkin @ 2013-01-25  9:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy5fi9vcz.fsf@alter.siamese.dyndns.org>

> Alexey Shumkin <alex.crezoff@gmail.com> writes:
> 
> > The expected SHA-1 digests are always available in variables.  Use
> > them instead of hardcoding.
> >
> > Signed-off-by: Alexey Shumkin <Alex.Crezoff@gmail.com>
> > ---
> 
> Looks good (" refactoring:" in the title may not want to be there,
> though).
oops, it's remained from "working version" after rebasing

> 
> Thanks.
> 
> >  t/t7102-reset.sh | 41 +++++++++++++++++++++--------------------
> >  1 file changed, 21 insertions(+), 20 deletions(-)
> >
> > diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
> > index b096dc8..cf492f4 100755
> > --- a/t/t7102-reset.sh
> > +++ b/t/t7102-reset.sh
> > @@ -28,7 +28,8 @@ test_expect_success 'creating initial files and
> > commits' ' 
> >  	echo "1st line 2nd file" >secondfile &&
> >  	echo "2nd line 2nd file" >>secondfile &&
> > -	git commit -a -m "modify 2nd file"
> > +	git commit -a -m "modify 2nd file" &&
> > +	head5=$(git rev-parse --verify HEAD)
> >  '
> >  # git log --pretty=oneline # to see those SHA1 involved
> >  
> > @@ -56,7 +57,7 @@ test_expect_success 'giving a non existing
> > revision should fail' ' test_must_fail git reset --mixed aaaaaa &&
> >  	test_must_fail git reset --soft aaaaaa &&
> >  	test_must_fail git reset --hard aaaaaa &&
> > -	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > +	check_changes $head5
> >  '
> >  
> >  test_expect_success 'reset --soft with unmerged index should fail'
> > ' @@ -74,7 +75,7 @@ test_expect_success \
> >  	test_must_fail git reset --hard -- first &&
> >  	test_must_fail git reset --soft HEAD^ -- first &&
> >  	test_must_fail git reset --hard HEAD^ -- first &&
> > -	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > +	check_changes $head5
> >  '
> >  
> >  test_expect_success 'giving unrecognized options should fail' '
> > @@ -86,7 +87,7 @@ test_expect_success 'giving unrecognized options
> > should fail' ' test_must_fail git reset --soft -o &&
> >  	test_must_fail git reset --hard --other &&
> >  	test_must_fail git reset --hard -o &&
> > -	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > +	check_changes $head5
> >  '
> >  
> >  test_expect_success \
> > @@ -110,7 +111,7 @@ test_expect_success \
> >  
> >  	git checkout master &&
> >  	git branch -D branch1 branch2 &&
> > -	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > +	check_changes $head5
> >  '
> >  
> >  test_expect_success \
> > @@ -133,27 +134,27 @@ test_expect_success \
> >  
> >  	git checkout master &&
> >  	git branch -D branch3 branch4 &&
> > -	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > +	check_changes $head5
> >  '
> >  
> >  test_expect_success \
> >  	'resetting to HEAD with no changes should succeed and do
> > nothing' ' git reset --hard &&
> > -		check_changes
> > 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
> > +		check_changes $head5 &&
> >  	git reset --hard HEAD &&
> > -		check_changes
> > 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
> > +		check_changes $head5 &&
> >  	git reset --soft &&
> > -		check_changes
> > 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
> > +		check_changes $head5 &&
> >  	git reset --soft HEAD &&
> > -		check_changes
> > 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
> > +		check_changes $head5 &&
> >  	git reset --mixed &&
> > -		check_changes
> > 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
> > +		check_changes $head5 &&
> >  	git reset --mixed HEAD &&
> > -		check_changes
> > 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
> > +		check_changes $head5 &&
> >  	git reset &&
> > -		check_changes
> > 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
> > +		check_changes $head5 &&
> >  	git reset HEAD &&
> > -		check_changes
> > 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > +		check_changes $head5
> >  '
> >  
> >  >.diff_expect
> > @@ -176,7 +177,7 @@ test_expect_success '--soft reset only should
> > show changes in diff --cached' ' git reset --soft HEAD^ &&
> >  	check_changes d1a4bc3abce4829628ae2dcb0d60ef3d1a78b1c4 &&
> >  	test "$(git rev-parse ORIG_HEAD)" = \
> > -			3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > +			$head5
> >  '
> >  
> >  >.diff_expect
> > @@ -193,7 +194,7 @@ test_expect_success \
> >  	git commit -a -C ORIG_HEAD &&
> >  	check_changes 3d3b7be011a58ca0c179ae45d94e6c83c0b0cd0d &&
> >  	test "$(git rev-parse ORIG_HEAD)" = \
> > -			3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > +			$head5
> >  '
> >  
> >  >.diff_expect
> > @@ -303,7 +304,7 @@ test_expect_success 'redoing the last two
> > commits should succeed' ' echo "1st line 2nd file" >secondfile &&
> >  	echo "2nd line 2nd file" >>secondfile &&
> >  	git commit -a -m "modify 2nd file" &&
> > -	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > +	check_changes $head5
> >  '
> >  
> >  >.diff_expect
> > @@ -341,15 +342,15 @@ EOF
> >  test_expect_success \
> >  	'--hard reset to ORIG_HEAD should clear a fast-forward
> > merge' ' git reset --hard HEAD^ &&
> > -	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
> > +	check_changes $head5 &&
> >  
> >  	git pull . branch1 &&
> >  	git reset --hard ORIG_HEAD &&
> > -	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc &&
> > +	check_changes $head5 &&
> >  
> >  	git checkout master &&
> >  	git branch -D branch1 branch2 &&
> > -	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
> > +	check_changes $head5
> >  '
> >  
> >  cat > expect << EOF

^ permalink raw reply

* Re: What's cooking in git.git (Jan 2013, #08; Tue, 22)
From: John Keeping @ 2013-01-25  9:09 UTC (permalink / raw)
  To: Chris Rorvick; +Cc: Junio C Hamano, git, Eric S. Raymond
In-Reply-To: <CAEUsAPagzN9wWAsSpBVOv7+ei3fAix407dB0EAUd7q7k7SugPw@mail.gmail.com>

On Thu, Jan 24, 2013 at 10:55:57PM -0600, Chris Rorvick wrote:
> On Wed, Jan 23, 2013 at 3:12 PM, John Keeping <john@keeping.me.uk> wrote:
> > The existing script (git-cvsimport.perl) won't ever work with cvsps-3
> > since features it relies on have been removed.
> 
> Not reporting the ancestry branch seems to be the big one.  Are there
> others?

For some reason I thought the non-fast-export output mode had already
been removed, but now that I check it looks like it's still there just
with a warning that it may be removed in the future.


John

^ 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  9:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v38xqba04.fsf@alter.siamese.dyndns.org>

> Alexey Shumkin <alex.crezoff@gmail.com> writes:
> 
> > The expected SHA-1 digests are always available in variables.  Use
> > them instead of hardcoding.
> >
> > Signed-off-by: Alexey Shumkin <Alex.Crezoff@gmail.com>
> > ---
> >  t/t6006-rev-list-format.sh | 130
> > +++++++++++++++++++++++++-------------------- 1 file changed, 72
> > insertions(+), 58 deletions(-)
> >
> > diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
> > index f94f0c4..c248509 100755
> > --- a/t/t6006-rev-list-format.sh
> > +++ b/t/t6006-rev-list-format.sh
> > @@ -6,8 +6,19 @@ test_description='git rev-list --pretty=format
> > test' 
> >  test_tick
> >  test_expect_success 'setup' '
> > -touch foo && git add foo && git commit -m "added foo" &&
> > -  echo changed >foo && git commit -a -m "changed foo"
> > +	touch foo &&
> 
> This is inherited from the original, but these days we try to avoid
> touch, unless it is about setting a new file timestamp.  The
> canonical (in the script we write) way to create an empty file is:
> 
>     : >foo
> 
> with or without ": ", it does not matter that much.
Ok!

> 
> > +	git add foo &&
> > +	git commit -m "added foo" &&
> > +	head1=$(git rev-parse --verify HEAD) &&
> > +	head1_7=$(echo $head1 | cut -c1-7) &&
> 
> Why do we want "whatever_7" variables and use "cut -c1-7" to produce
> them?  Is "7" something we care deeply about?
I did not spend too much time to think of names of variables at the
moment I was writing this code )
> 
> 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?
It's an inherited code from 1.5 years ago Git ;) taken from some other
tests
I'll change it as you propose )

> 
> > +	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.
yep! you're definitely right

> 
> > +	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.
> 
> > @@ -131,39 +142,42 @@ This commit message is much longer than the
> > others, and it will be encoded in iso8859-1. We should therefore
> >  include an iso8859 character: ¡bueno!
> >  EOF
> > +
> >  test_expect_success 'setup complex body' '
> > -git config i18n.commitencoding iso8859-1 &&
> > -  echo change2 >foo && git commit -a -F commit-msg
> > +	git config i18n.commitencoding iso8859-1 &&
> > +	echo change2 >foo && git commit -a -F commit-msg &&
> > +	head3=$(git rev-parse --verify HEAD) &&
> > +	head3_7=$(echo $head3 | cut -c1-7)
> >  '
> 
> Likewise.
> 
> Thanks.

^ permalink raw reply

* 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

* [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

* [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 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 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 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 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 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 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

* 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

* 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: 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 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


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