{"thread":{"id":"37705","subject":"[PATCH v2 1/2] mergetool: don't require a work tree for --tool-help","startedAt":"2014-10-10T08:51:43Z","lastAt":"2014-10-11T08:35:51Z","messageCount":5,"participants":["David Aguilar","Charles Bailey"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"250476","messageId":"1412931104-74648-1-git-send-email-davvid@gmail.com","threadId":"37705","inReplyTo":null,"subject":"[PATCH v2 1/2] mergetool: don't require a work tree for --tool-help","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-10-10T08:51:43Z","receivedAt":"2014-10-10T08:51:43Z","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 v1:\n\nNONGIT_OK=Yes was added to make it actually work outside of a git repo.\n\n git-mergetool.sh | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/git-mergetool.sh b/git-mergetool.sh\nindex 96a61ba..a502430 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,8 @@ prompt_after_failed_merge () {\n \tdone\n }\n \n+require_work_tree\n+\n if test -z \"$merge_tool\"\n then\n \t# Check if a merge tool has been configured\n-- \n2.1.2.377.g0e18f12\n"},{"id":"250477","messageId":"1412931104-74648-2-git-send-email-davvid@gmail.com","threadId":"37705","inReplyTo":"1412931104-74648-1-git-send-email-davvid@gmail.com","subject":"[PATCH v2 2/2] difftool: don't assume that default sh is sane","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-10-10T08:51:44Z","receivedAt":"2014-10-10T08:51:44Z","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---\nChanges since v1:\n\nThe case statement for --tool-help=* was made simpler\nthanks to John.\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 a502430..ea8fe37 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.377.g0e18f12\n"},{"id":"250479","messageId":"FCAAE5E4-E220-4B45-A6A1-B2AFB79F9069@hashpling.org","threadId":"37705","inReplyTo":"1412931104-74648-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH v2 1/2] mergetool: don't require a work tree for --tool-help","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2014-10-10T09:01:57Z","receivedAt":"2014-10-10T09:01:57Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"On 10 Oct 2014, at 09:51, David Aguilar wrote:\n> Changes since v1:\n> \n> NONGIT_OK=Yes was added to make it actually work outside of a git repo.\n\nDoes this actually work? The reason that I haven't got around to resending my re-roll is that I found that I needed changes to git-sh-setup.sh because doing NONGIT_OK and then require_work_tree didn't correctly set GIT_DIR when it wasn't already explicitly set in the environment. (I believe the rest of mergetool relies on it.)\n\nPerhaps I misunderstood, though."},{"id":"250499","messageId":"20141011082954.GA79450@gmail.com","threadId":"37705","inReplyTo":"FCAAE5E4-E220-4B45-A6A1-B2AFB79F9069@hashpling.org","subject":"Re: [PATCH v2 1/2] mergetool: don't require a work tree for --tool-help","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-10-11T08:29:56Z","receivedAt":"2014-10-11T08:29:56Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Fri, Oct 10, 2014 at 10:01:57AM +0100, Charles Bailey wrote:\n> On 10 Oct 2014, at 09:51, David Aguilar wrote:\n> > Changes since v1:\n> > \n> > NONGIT_OK=Yes was added to make it actually work outside of a git repo.\n> \n> Does this actually work? The reason that I haven't got around\n> to resending my re-roll is that I found that I needed changes\n> to git-sh-setup.sh because doing NONGIT_OK and then\n> require_work_tree didn't correctly set GIT_DIR when it wasn't\n> already explicitly set in the environment. (I believe the rest\n> of mergetool relies on it.)\n> \n> Perhaps I misunderstood, though.\n\nThanks for the heads-up.\n\nI tested mergetool and it seems fine but indeed there's an\n`if test -e \"$GIT_DIR/MERGE_RR\"` in there that is surely not\nworking as intended.\n\nOne solution would be to move the work done in the test -z \"$NONGIT_OK\"\nblock in git-sh-setup into a function e.g. git_dir_init () so\nthat we can defer the GIT_DIR initialization until after\nrequire_work_tree has been called.\n-- \nDavid\n"},{"id":"250500","messageId":"3D01E1E7-627C-427F-9E99-4B2271447B0D@hashpling.org","threadId":"37705","inReplyTo":"20141011082954.GA79450@gmail.com","subject":"Re: [PATCH v2 1/2] mergetool: don't require a work tree for --tool-help","fromName":"Charles Bailey","fromEmail":"charles@hashpling.org","sentAt":"2014-10-11T08:35:51Z","receivedAt":"2014-10-11T08:35:51Z","isPatch":true,"sender":{"key":"charles@hashpling.org","avatar":"https://avatars.githubusercontent.com/u/1668475?v=4"},"body":"\n\n\n> On 11 Oct 2014, at 09:29, David Aguilar wrote:\n> \n> Thanks for the heads-up.\n> \n> I tested mergetool and it seems fine but indeed there's an\n> `if test -e \"$GIT_DIR/MERGE_RR\"` in there that is surely not\n> working as intended.\n> \n> One solution would be to move the work done in the test -z \"$NONGIT_OK\"\n> block in git-sh-setup into a function e.g. git_dir_init () so\n> that we can defer the GIT_DIR initialization until after\n> require_work_tree has been called.\n\nI believe I had a very similar idea but the vast number of things that would potentially be affected by changing git-sh-setup.sh made me put things on hold in case I had any other ideas.\n\nI haven't so I think this is probably the best approach."}]}