{"thread":{"id":"37763","subject":"[PATCH] Copy mergetool \"bc3\" as \"bc4\"","startedAt":"2014-10-20T11:03:37Z","lastAt":"2014-10-21T18:27:53Z","messageCount":8,"participants":["Olivier Croquette","Junio C Hamano","Sebastian Schuberth","David Aguilar"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"250852","messageId":"1413803017-30489-1-git-send-email-ocroquette@free.fr","threadId":"37763","inReplyTo":null,"subject":"[PATCH] Copy mergetool \"bc3\" as \"bc4\"","fromName":"Olivier Croquette","fromEmail":"ocroquette@free.fr","sentAt":"2014-10-20T11:03:37Z","receivedAt":"2014-10-20T11:03:37Z","isPatch":true,"sender":{"key":"ocroquette@free.fr","avatar":null},"body":"Beyond compare 4 is out since september 2014. The CLI interface doesn't seem to have changed compared to the version 3.\n\nSigned-off-by: Olivier Croquette \n---\n mergetools/bc4 | 25 +++++++++++++++++++++++++\n 1 files changed, 25 insertions(+), 0 deletions(-)\n create mode 100644 mergetools/bc4\n\ndiff --git a/mergetools/bc4 b/mergetools/bc4\nnew file mode 100644\nindex 0000000..b6319d2\n--- /dev/null\n+++ b/mergetools/bc4\n@@ -0,0 +1,25 @@\n+diff_cmd () {\n+\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n+}\n+\n+merge_cmd () {\n+\ttouch \"$BACKUP\"\n+\tif $base_present\n+\tthen\n+\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \"$BASE\" \\\n+\t\t\t-mergeoutput=\"$MERGED\"\n+\telse\n+\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n+\t\t\t-mergeoutput=\"$MERGED\"\n+\tfi\n+\tcheck_unchanged\n+}\n+\n+translate_merge_tool_path() {\n+\tif type bcomp >/dev/null 2>/dev/null\n+\tthen\n+\t\techo bcomp\n+\telse\n+\t\techo bcompare\n+\tfi\n+}\n-- \n1.7.2.5\n"},{"id":"250859","messageId":"xmqq7fzu4rve.fsf@gitster.dls.corp.google.com","threadId":"37763","inReplyTo":"1413803017-30489-1-git-send-email-ocroquette@free.fr","subject":"Re: [PATCH] Copy mergetool \"bc3\" as \"bc4\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-20T16:40:05Z","receivedAt":"2014-10-20T16:40:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Olivier Croquette writes:\n\n> Beyond compare 4 is out since september 2014. The CLI interface\n> doesn't seem to have changed compared to the version 3.\n\nHmph, if this is identical to mergetools/bc3, why is the patch even\nneeded? Do we auto-detect something and try to use bc4 which does\nnot exist and fail, and we must supply a copy as bc4 to prevent it?\n\nIt may feel somewhat strange to have to say \"mergetool --tool=bc3\"\nwhen you know what you have is version 4 and not version 3, but in\nthat case, I wonder if there are reasons why calling both versions\njust \"bc\" is a bad idea. And assuming that version 5 and later\nwould keep the same interface, we will not have to worry about that\n\"I have version 7 why do I have to say 3?\" if we can go that route.\n\nPerhaps version 2 and before are unusable as a mergetool backend or\nsomething?\n\n>\n> Signed-off-by: Olivier Croquette \n> ---\n> mergetools/bc4 | 25 +++++++++++++++++++++++++\n> 1 files changed, 25 insertions(+), 0 deletions(-)\n> create mode 100644 mergetools/bc4\n>\n> diff --git a/mergetools/bc4 b/mergetools/bc4\n> new file mode 100644\n> index 0000000..b6319d2\n> --- /dev/null\n> +++ b/mergetools/bc4\n> @@ -0,0 +1,25 @@\n> +diff_cmd () {\n> +\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n> +}\n> +\n> +merge_cmd () {\n> +\ttouch \"$BACKUP\"\n> +\tif $base_present\n> +\tthen\n> +\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \"$BASE\" \\\n> +\t\t\t-mergeoutput=\"$MERGED\"\n> +\telse\n> +\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n> +\t\t\t-mergeoutput=\"$MERGED\"\n> +\tfi\n> +\tcheck_unchanged\n> +}\n> +\n> +translate_merge_tool_path() {\n> +\tif type bcomp >/dev/null 2>/dev/null\n> +\tthen\n> +\t\techo bcomp\n> +\telse\n> +\t\techo bcompare\n> +\tfi\n> +}\n"},{"id":"250863","messageId":"xmqqtx2y3avx.fsf@gitster.dls.corp.google.com","threadId":"37763","inReplyTo":"xmqq7fzu4rve.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Copy mergetool \"bc3\" as \"bc4\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-20T17:32:18Z","receivedAt":"2014-10-20T17:32:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Olivier Croquette writes:\n>\n>> Beyond compare 4 is out since september 2014. The CLI interface\n>> doesn't seem to have changed compared to the version 3.\n>\n> Hmph, if this is identical to mergetools/bc3, why is the patch even\n> needed? Do we auto-detect something and try to use bc4 which does\n> not exist and fail, and we must supply a copy as bc4 to prevent it?\n>\n> It may feel somewhat strange to have to say \"mergetool --tool=bc3\"\n> when you know what you have is version 4 and not version 3, but in\n> that case, I wonder if there are reasons why calling both versions\n> just \"bc\" is a bad idea. And assuming that version 5 and later\n> would keep the same interface, we will not have to worry about that\n> \"I have version 7 why do I have to say 3?\" if we can go that route.\n>\n> Perhaps version 2 and before are unusable as a mergetool backend or\n> something?\n\nIt seems that ffe6dc08 (mergetool--lib: Add Beyond Compare 3 as a\ntool, 2011-02-27) is the first mention of \"Beyond Compare\" and it\nonly was interested in version 3 and nothing else.\n\nPerhaps something like this, so that existing users can still use\n\"bc3\" and other people can use \"bc\" if it bothers them that they\nhave to say \"3\" when the backend driver works with both 3 and 4?\n\n---\n contrib/completion/git-completion.bash | 2 +-\n git-mergetool--lib.sh | 2 +-\n mergetools/{bc3 => bc} | 0\n mergetools/bc3 | 26 +-------------------------\n 4 files changed, 3 insertions(+), 27 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex d548e99..01259cc 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1203,7 +1203,7 @@ _git_diff ()\n }\n \n __git_mergetools_common=\"diffuse diffmerge ecmerge emerge kdiff3 meld opendiff\n-\t\t\ttkdiff vimdiff gvimdiff xxdiff araxis p4merge bc3 codecompare\n+\t\t\ttkdiff vimdiff gvimdiff xxdiff araxis p4merge bc codecompare\n \"\n \n _git_difftool ()\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex c45a020..1d8a26d 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -250,7 +250,7 @@ list_merge_tool_candidates () {\n \t\t\ttools=\"opendiff kdiff3 tkdiff xxdiff meld $tools\"\n \t\tfi\n \t\ttools=\"$tools gvimdiff diffuse diffmerge ecmerge\"\n-\t\ttools=\"$tools p4merge araxis bc3 codecompare\"\n+\t\ttools=\"$tools p4merge araxis bc bc3 codecompare\"\n \tfi\n \tcase \"${VISUAL:-$EDITOR}\" in\n \t*vim*)\ndiff --git a/mergetools/bc3 b/mergetools/bc\nsimilarity index 100%\nrename from mergetools/bc3\nrename to mergetools/bc\ndiff --git a/mergetools/bc3 b/mergetools/bc3\ndissimilarity index 100%\nindex b6319d2..5d8dd48 100644\n--- a/mergetools/bc3\n+++ b/mergetools/bc3\n@@ -1,25 +1 @@\n-diff_cmd () {\n-\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n-}\n-\n-merge_cmd () {\n-\ttouch \"$BACKUP\"\n-\tif $base_present\n-\tthen\n-\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \"$BASE\" \\\n-\t\t\t-mergeoutput=\"$MERGED\"\n-\telse\n-\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t-mergeoutput=\"$MERGED\"\n-\tfi\n-\tcheck_unchanged\n-}\n-\n-translate_merge_tool_path() {\n-\tif type bcomp >/dev/null 2>/dev/null\n-\tthen\n-\t\techo bcomp\n-\telse\n-\t\techo bcompare\n-\tfi\n-}\n+. \"$MERGE_TOOLS_DIR/bc\"\n"},{"id":"250864","messageId":"54455334.8000503@gmail.com","threadId":"37763","inReplyTo":"xmqqtx2y3avx.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Copy mergetool \"bc3\" as \"bc4\"","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2014-10-20T18:23:48Z","receivedAt":"2014-10-20T18:23:48Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On 20.10.2014 19:32, Junio C Hamano wrote:\n\n>>> Beyond compare 4 is out since september 2014. The CLI interface\n>>> doesn't seem to have changed compared to the version 3.\n>>\n>> Hmph, if this is identical to mergetools/bc3, why is the patch even\n>> needed? Do we auto-detect something and try to use bc4 which does\n>> not exist and fail, and we must supply a copy as bc4 to prevent it?\n\nThe patch is indeed not needed, which is why I haven't cared to provide it so far although I'm now using Beyond Compare 4 instead of version 3 myself.\n\n>> It may feel somewhat strange to have to say \"mergetool --tool=bc3\"\n>> when you know what you have is version 4 and not version 3, but in\n\nThat's exactly the only reason I could think of why it could be nice to have a \"bc4\".\n\n>> that case, I wonder if there are reasons why calling both versions\n>> just \"bc\" is a bad idea. And assuming that version 5 and later\n\nIMHO, the only reason not to just have a single \"bc\" is to maintain backward compatibility for users already using \"bc3\". But for the sake of cleaner code, personally I'd be fine with that minor backward compatibility breakage.\n\n>> Perhaps version 2 and before are unusable as a mergetool backend or\n>> something?\n> \n> It seems that ffe6dc08 (mergetool--lib: Add Beyond Compare 3 as a\n> tool, 2011-02-27) is the first mention of \"Beyond Compare\" and it\n> only was interested in version 3 and nothing else.\n\nBeyond Compare versions prior to 3 do not run on Linux, but only on Windows, which is why I did not care to submit a patch.\n\n> Perhaps something like this, so that existing users can still use\n> \"bc3\" and other people can use \"bc\" if it bothers them that they\n> have to say \"3\" when the backend driver works with both 3 and 4?\n\nThat indeed sounds like the best approach.\n\n> --- a/git-mergetool--lib.sh\n> +++ b/git-mergetool--lib.sh\n> @@ -250,7 +250,7 @@ list_merge_tool_candidates () {\n> \t\t\ttools=\"opendiff kdiff3 tkdiff xxdiff meld $tools\"\n> \t\tfi\n> \t\ttools=\"$tools gvimdiff diffuse diffmerge ecmerge\"\n> -\t\ttools=\"$tools p4merge araxis bc3 codecompare\"\n> +\t\ttools=\"$tools p4merge araxis bc bc3 codecompare\"\n\nWhy keep bc3 here?\n\nAnd shouldn't we update git-gui/lib/mergetool.tcl, too?\n\n-- \nSebastian Schuberth\n"},{"id":"250868","messageId":"xmqqfvei37qg.fsf@gitster.dls.corp.google.com","threadId":"37763","inReplyTo":"54455334.8000503@gmail.com","subject":"Re: [PATCH] Copy mergetool \"bc3\" as \"bc4\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-20T18:40:23Z","receivedAt":"2014-10-20T18:40:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sebastian Schuberth writes:\n\n>> Perhaps something like this, so that existing users can still use\n>> \"bc3\" and other people can use \"bc\" if it bothers them that they\n>> have to say \"3\" when the backend driver works with both 3 and 4?\n>\n> That indeed sounds like the best approach.\n>\n>> --- a/git-mergetool--lib.sh\n>> +++ b/git-mergetool--lib.sh\n>> @@ -250,7 +250,7 @@ list_merge_tool_candidates () {\n>> \t\t\ttools=\"opendiff kdiff3 tkdiff xxdiff meld $tools\"\n>> \t\tfi\n>> \t\ttools=\"$tools gvimdiff diffuse diffmerge ecmerge\"\n>> -\t\ttools=\"$tools p4merge araxis bc3 codecompare\"\n>> +\t\ttools=\"$tools p4merge araxis bc bc3 codecompare\"\n>\n> Why keep bc3 here?\n\nI didn't carefully look at the code that uses this list to see if we\nhave to list everything or can list just the ones we recommend, and\nerred on the safer side (unlike the one for completion where I\nomitted bc3 as \"deprecated\").\n\nI'll let mergetools experts decide when rolling the final patch ;-)\n\n> And shouldn't we update git-gui/lib/mergetool.tcl, too?\n\nYes we should, but git-gui is not in my bailiwick, and shouldn't be\ndone relative to my tree anyway. I'll Cc this message to Pat.\n\nThanks.\n"},{"id":"250874","messageId":"2D3655AD-CC8C-41C2-B0B7-4803176CFB31@free.fr","threadId":"37763","inReplyTo":"xmqqtx2y3avx.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Copy mergetool \"bc3\" as \"bc4\"","fromName":"Olivier Croquette","fromEmail":"ocroquette@free.fr","sentAt":"2014-10-20T20:35:51Z","receivedAt":"2014-10-20T20:35:51Z","isPatch":true,"sender":{"key":"ocroquette@free.fr","avatar":null},"body":"On 20 Oct 2014, at 19:32, Junio C Hamano wrote:\n> Perhaps something like this, so that existing users can still use\n> \"bc3\" and other people can use \"bc\" if it bothers them that they\n> have to say \"3\" when the backend driver works with both 3 and 4?\n\nThanks for the quick and great feedback.\nThis looks good to me. I used “bc4\" just for consistency sake and to preserve backwards compatibility, but introducing “bc” as a generic backend looks indeed as a better approach.\nIt’s even future safe, because it doesn’t prevent introducing bc5 if required later on.\n\nOlivier\n"},{"id":"250890","messageId":"20141021084436.GA39148@gmail.com","threadId":"37763","inReplyTo":"xmqqfvei37qg.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] Copy mergetool \"bc3\" as \"bc4\"","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2014-10-21T08:44:38Z","receivedAt":"2014-10-21T08:44:38Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Mon, Oct 20, 2014 at 11:40:23AM -0700, Junio C Hamano wrote:\n> Sebastian Schuberth writes:\n> \n> >> Perhaps something like this, so that existing users can still use\n> >> \"bc3\" and other people can use \"bc\" if it bothers them that they\n> >> have to say \"3\" when the backend driver works with both 3 and 4?\n> >\n> > That indeed sounds like the best approach.\n> >\n> >> --- a/git-mergetool--lib.sh\n> >> +++ b/git-mergetool--lib.sh\n> >> @@ -250,7 +250,7 @@ list_merge_tool_candidates () {\n> >> \t\t\ttools=\"opendiff kdiff3 tkdiff xxdiff meld $tools\"\n> >> \t\tfi\n> >> \t\ttools=\"$tools gvimdiff diffuse diffmerge ecmerge\"\n> >> -\t\ttools=\"$tools p4merge araxis bc3 codecompare\"\n> >> +\t\ttools=\"$tools p4merge araxis bc bc3 codecompare\"\n> >\n> > Why keep bc3 here?\n> \n> I didn't carefully look at the code that uses this list to see if we\n> have to list everything or can list just the ones we recommend, and\n> erred on the safer side (unlike the one for completion where I\n> omitted bc3 as \"deprecated\").\n\nWe should drop \"bc3\" here. This is the list of candidates that\nis used when no configuration exists. Listing \"bc\" twice here\nimplies that we might try that candidate twice.\n\nOtherwise, this patch looks like the right way to go ~ it makes\n\"bc\" available and keeps compatibility for \"bc3\".\n\nIf we wanted to phase out \"bc3\" for Git 3.0 we could start\nwarning inside of the \"bc3\" scriptlet, but I don't see any\nreason to do so a.t.m.\n\nThanks,\n-- \nDavid\n"},{"id":"250905","messageId":"xmqqsiihz39y.fsf@gitster.dls.corp.google.com","threadId":"37763","inReplyTo":"20141021084436.GA39148@gmail.com","subject":"Re: [PATCH] Copy mergetool \"bc3\" as \"bc4\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-10-21T18:27:53Z","receivedAt":"2014-10-21T18:27:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n> Otherwise, this patch looks like the right way to go ~ it makes\n> \"bc\" available and keeps compatibility for \"bc3\".\n\nOK, thanks. Here is what I'll queue.\n\n-- >8 --\nFrom: Junio C Hamano \nDate: Mon, 20 Oct 2014 15:49:36 -0700\nSubject: [PATCH] mergetool: rename bc3 to bc\n\nBeyond Compare version 4 works the same way as version 3, so rename\nthe existing \"bc3\" adaptor to just \"bc\", while keeping \"bc3\" as a\nbackward compatible wrapper.\n\nNoticed-by: Olivier Croquette \nHelped-by: David Aguilar \nSigned-off-by: Junio C Hamano \n---\n contrib/completion/git-completion.bash | 2 +-\n git-mergetool--lib.sh | 2 +-\n mergetools/bc | 25 +++++++++++++++++++++++++\n mergetools/bc3 | 26 +-------------------------\n 4 files changed, 28 insertions(+), 27 deletions(-)\n create mode 100644 mergetools/bc\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 06bf262..8a19b3e 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1207,7 +1207,7 @@ _git_diff ()\n }\n \n __git_mergetools_common=\"diffuse diffmerge ecmerge emerge kdiff3 meld opendiff\n-\t\t\ttkdiff vimdiff gvimdiff xxdiff araxis p4merge bc3 codecompare\n+\t\t\ttkdiff vimdiff gvimdiff xxdiff araxis p4merge bc codecompare\n \"\n \n _git_difftool ()\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex c45a020..a40d3df 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -250,7 +250,7 @@ list_merge_tool_candidates () {\n \t\t\ttools=\"opendiff kdiff3 tkdiff xxdiff meld $tools\"\n \t\tfi\n \t\ttools=\"$tools gvimdiff diffuse diffmerge ecmerge\"\n-\t\ttools=\"$tools p4merge araxis bc3 codecompare\"\n+\t\ttools=\"$tools p4merge araxis bc codecompare\"\n \tfi\n \tcase \"${VISUAL:-$EDITOR}\" in\n \t*vim*)\ndiff --git a/mergetools/bc b/mergetools/bc\nnew file mode 100644\nindex 0000000..b6319d2\n--- /dev/null\n+++ b/mergetools/bc\n@@ -0,0 +1,25 @@\n+diff_cmd () {\n+\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n+}\n+\n+merge_cmd () {\n+\ttouch \"$BACKUP\"\n+\tif $base_present\n+\tthen\n+\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \"$BASE\" \\\n+\t\t\t-mergeoutput=\"$MERGED\"\n+\telse\n+\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n+\t\t\t-mergeoutput=\"$MERGED\"\n+\tfi\n+\tcheck_unchanged\n+}\n+\n+translate_merge_tool_path() {\n+\tif type bcomp >/dev/null 2>/dev/null\n+\tthen\n+\t\techo bcomp\n+\telse\n+\t\techo bcompare\n+\tfi\n+}\ndiff --git a/mergetools/bc3 b/mergetools/bc3\nindex b6319d2..5d8dd48 100644\n--- a/mergetools/bc3\n+++ b/mergetools/bc3\n@@ -1,25 +1 @@\n-diff_cmd () {\n-\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\"\n-}\n-\n-merge_cmd () {\n-\ttouch \"$BACKUP\"\n-\tif $base_present\n-\tthen\n-\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \"$BASE\" \\\n-\t\t\t-mergeoutput=\"$MERGED\"\n-\telse\n-\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t-mergeoutput=\"$MERGED\"\n-\tfi\n-\tcheck_unchanged\n-}\n-\n-translate_merge_tool_path() {\n-\tif type bcomp >/dev/null 2>/dev/null\n-\tthen\n-\t\techo bcomp\n-\telse\n-\t\techo bcompare\n-\tfi\n-}\n+. \"$MERGE_TOOLS_DIR/bc\"\n-- \n2.1.2-583-g325e495\n"}]}