{"thread":{"id":"37710","subject":"[PATCH v3 1/3] git-sh-setup: move GIT_DIR initialization into a function","startedAt":"2014-10-11T08:39:36Z","lastAt":"2014-10-15T06:48:26Z","messageCount":6,"participants":["David Aguilar","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":3},"messages":[{"id":"250501","messageId":"1413016778-94829-1-git-send-email-davvid@gmail.com","threadId":"37710","inReplyTo":null,"subject":"[PATCH v3 1/3] git-sh-setup: move GIT_DIR initialization into a function","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-10-11T08:39:36Z","receivedAt":"2014-10-11T08:39:36Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Signed-off-by: David Aguilar \n---\nThis is a new patch since the last round, prep for 2/3\n\n git-sh-setup.sh | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/git-sh-setup.sh b/git-sh-setup.sh\nindex 9447980..d968760 100644\n--- a/git-sh-setup.sh\n+++ b/git-sh-setup.sh\n@@ -330,8 +330,7 @@ esac\n \n # Make sure we are in a valid repository of a vintage we understand,\n # if we require to be in a git repository.\n-if test -z \"$NONGIT_OK\"\n-then\n+git_dir_init () {\n \tGIT_DIR=$(git rev-parse --git-dir) || exit\n \tif [ -z \"$SUBDIRECTORY_OK\" ]\n \tthen\n@@ -346,6 +345,11 @@ then\n \t\texit 1\n \t}\n \t: ${GIT_OBJECT_DIRECTORY=\"$GIT_DIR/objects\"}\n+}\n+\n+if test -z \"$NONGIT_OK\"\n+then\n+\tgit_dir_init\n fi\n \n peel_committish () {\n-- \n2.1.2.378.g89c0b73\n"},{"id":"250502","messageId":"1413016778-94829-2-git-send-email-davvid@gmail.com","threadId":"37710","inReplyTo":"1413016778-94829-1-git-send-email-davvid@gmail.com","subject":"[PATCH v3 2/3] mergetool: don't require a work tree for --tool-help","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-10-11T08:39:37Z","receivedAt":"2014-10-11T08:39:37Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"From: Charles Bailey \n\nSigned-off-by: Charles Bailey \nSigned-off-by: David Aguilar \n---\nChanges since v2:\n\nThis now uses the new git_dir_init function.\n\n git-mergetool.sh | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 96a61ba..cddb533 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -10,11 +10,11 @@\n \n USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] ...'\n SUBDIRECTORY_OK=Yes\n+NONGIT_OK=Yes\n OPTIONS_SPEC=\n TOOL_MODE=merge\n . git-sh-setup\n . git-mergetool--lib\n-require_work_tree\n \n # Returns true if the mode reflects a symlink\n is_symlink () {\n@@ -378,6 +378,9 @@ prompt_after_failed_merge () {\n \tdone\n }\n \n+require_work_tree\n+git_dir_init\n+\n if test -z \"$merge_tool\"\n then\n \t# Check if a merge tool has been configured\n-- \n2.1.2.378.g89c0b73\n"},{"id":"250503","messageId":"1413016778-94829-3-git-send-email-davvid@gmail.com","threadId":"37710","inReplyTo":"1413016778-94829-2-git-send-email-davvid@gmail.com","subject":"[PATCH v3 3/3] difftool: don't assume that default sh is sane","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-10-11T08:39:38Z","receivedAt":"2014-10-11T08:39:38Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"From: Charles Bailey \n\ngit-difftool used to create a command list script containing $( ... )\nand explicitly calls \"sh -c\" with this list.\n\nInstead, allow mergetool --tool-help to take a mode parameter and call\nmergetool directly to invoke the show_tool_help function. This mode\nparameter is intented for use solely by difftool.\n\nSigned-off-by: Charles Bailey \nHelped-by: John Keeping \nSigned-off-by: David Aguilar \n---\nNo changes since v2.\n\n git-difftool.perl | 6 +-----\n git-mergetool.sh | 4 ++++\n 2 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/git-difftool.perl b/git-difftool.perl\nindex 18ca61e..598fcc2 100755\n--- a/git-difftool.perl\n+++ b/git-difftool.perl\n@@ -47,13 +47,9 @@ sub find_worktree\n \n sub print_tool_help\n {\n-\tmy $cmd = 'TOOL_MODE=diff';\n-\t$cmd .= ' && . \"$(git --exec-path)/git-mergetool--lib\"';\n-\t$cmd .= ' && show_tool_help';\n-\n \t# See the comment at the bottom of file_diff() for the reason behind\n \t# using system() followed by exit() instead of exec().\n-\tmy $rc = system('sh', '-c', $cmd);\n+\tmy $rc = system(qw(git mergetool --tool-help=diff));\n \texit($rc | ($rc >> 8));\n }\n \ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex cddb533..10782b8 100755\n--- a/git-mergetool.sh\n+++ b/git-mergetool.sh\n@@ -327,6 +327,10 @@ guessed_merge_tool=false\n while test $# != 0\n do\n \tcase \"$1\" in\n+\t--tool-help=*)\n+\t\tTOOL_MODE=${1#--tool-help=}\n+\t\tshow_tool_help\n+\t\t;;\n \t--tool-help)\n \t\tshow_tool_help\n \t\t;;\n-- \n2.1.2.378.g89c0b73\n"},{"id":"250567","messageId":"xmqqsiir3hlk.fsf@gitster.dls.corp.google.com","threadId":"37710","inReplyTo":"1413016778-94829-2-git-send-email-davvid@gmail.com","subject":"Re: [PATCH v3 2/3] mergetool: don't require a work tree for --tool-help","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-13T19:16:55Z","receivedAt":"2014-10-13T19:16:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n> From: Charles Bailey \n>\n> Signed-off-by: Charles Bailey \n> Signed-off-by: David Aguilar \n> ---\n> Changes since v2:\n>\n> This now uses the new git_dir_init function.\n>\n> git-mergetool.sh | 5 ++++-\n> 1 file changed, 4 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-mergetool.sh b/git-mergetool.sh\n> index 96a61ba..cddb533 100755\n> --- a/git-mergetool.sh\n> +++ b/git-mergetool.sh\n> @@ -10,11 +10,11 @@\n> \n> USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] ...'\n> SUBDIRECTORY_OK=Yes\n> +NONGIT_OK=Yes\n> OPTIONS_SPEC=\n> TOOL_MODE=merge\n> . git-sh-setup\n> . git-mergetool--lib\n> -require_work_tree\n> \n> # Returns true if the mode reflects a symlink\n> is_symlink () {\n> @@ -378,6 +378,9 @@ prompt_after_failed_merge () {\n> \tdone\n> }\n> \n> +require_work_tree\n> +git_dir_init\n\nThis is somewhat curious. Shouldn't the order of these swapped?\n\n> +\n> if test -z \"$merge_tool\"\n> then\n> \t# Check if a merge tool has been configured\n"},{"id":"250664","messageId":"20141015063509.GB14751@gmail.com","threadId":"37710","inReplyTo":"xmqqsiir3hlk.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3 2/3] mergetool: don't require a work tree for --tool-help","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-10-15T06:35:11Z","receivedAt":"2014-10-15T06:35:11Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Mon, Oct 13, 2014 at 12:16:55PM -0700, Junio C Hamano wrote:\n> David Aguilar writes:\n> \n> > From: Charles Bailey \n> >\n> > Signed-off-by: Charles Bailey \n> > Signed-off-by: David Aguilar \n> > ---\n> > Changes since v2:\n> >\n> > This now uses the new git_dir_init function.\n> >\n> > git-mergetool.sh | 5 ++++-\n> > 1 file changed, 4 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/git-mergetool.sh b/git-mergetool.sh\n> > index 96a61ba..cddb533 100755\n> > --- a/git-mergetool.sh\n> > +++ b/git-mergetool.sh\n> > @@ -10,11 +10,11 @@\n> > \n> > USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] ...'\n> > SUBDIRECTORY_OK=Yes\n> > +NONGIT_OK=Yes\n> > OPTIONS_SPEC=\n> > TOOL_MODE=merge\n> > . git-sh-setup\n> > . git-mergetool--lib\n> > -require_work_tree\n> > \n> > # Returns true if the mode reflects a symlink\n> > is_symlink () {\n> > @@ -378,6 +378,9 @@ prompt_after_failed_merge () {\n> > \tdone\n> > }\n> > \n> > +require_work_tree\n> > +git_dir_init\n> \n> This is somewhat curious. Shouldn't the order of these swapped?\n\nYes. I'll send a replacement patch for 2/3 only.\n-- \nDavid\n"},{"id":"250666","messageId":"20141015064824.GD14751@gmail.com","threadId":"37710","inReplyTo":"20141015063509.GB14751@gmail.com","subject":"Re: [PATCH v3 2/3] mergetool: don't require a work tree for --tool-help","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-10-15T06:48:26Z","receivedAt":"2014-10-15T06:48:26Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Tue, Oct 14, 2014 at 11:35:11PM -0700, David Aguilar wrote:\n> On Mon, Oct 13, 2014 at 12:16:55PM -0700, Junio C Hamano wrote:\n> > David Aguilar writes:\n> > \n> > > From: Charles Bailey \n> > >\n> > > Signed-off-by: Charles Bailey \n> > > Signed-off-by: David Aguilar \n> > > ---\n> > > Changes since v2:\n> > >\n> > > This now uses the new git_dir_init function.\n> > >\n> > > git-mergetool.sh | 5 ++++-\n> > > 1 file changed, 4 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/git-mergetool.sh b/git-mergetool.sh\n> > > index 96a61ba..cddb533 100755\n> > > --- a/git-mergetool.sh\n> > > +++ b/git-mergetool.sh\n> > > @@ -10,11 +10,11 @@\n> > > \n> > > USAGE='[--tool=tool] [--tool-help] [-y|--no-prompt|--prompt] [file to merge] ...'\n> > > SUBDIRECTORY_OK=Yes\n> > > +NONGIT_OK=Yes\n> > > OPTIONS_SPEC=\n> > > TOOL_MODE=merge\n> > > . git-sh-setup\n> > > . git-mergetool--lib\n> > > -require_work_tree\n> > > \n> > > # Returns true if the mode reflects a symlink\n> > > is_symlink () {\n> > > @@ -378,6 +378,9 @@ prompt_after_failed_merge () {\n> > > \tdone\n> > > }\n> > > \n> > > +require_work_tree\n> > > +git_dir_init\n> > \n> > This is somewhat curious. Shouldn't the order of these swapped?\n> \n> Yes. I'll send a replacement patch for 2/3 only.\n\nNevermind, I noticed you already fixed this up in pu.\n\nThank you,\n-- \nDavid\n"}]}