Re: [PATCH] Copy mergetool bc3 as bc4
On Mon, Oct 20, 2014 at 11:40:23AM -0700, Junio C Hamano wrote: Sebastian Schuberth sschube...@gmail.com writes: Perhaps something like this, so that existing users can still use bc3 and other people can use bc if it bothers them that they have to say 3 when the backend driver works with both 3 and 4? That indeed sounds like the best approach. --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -250,7 +250,7 @@ list_merge_tool_candidates () { tools=opendiff kdiff3 tkdiff xxdiff meld $tools fi tools=$tools gvimdiff diffuse diffmerge ecmerge - tools=$tools p4merge araxis bc3 codecompare + tools=$tools p4merge araxis bc bc3 codecompare Why keep bc3 here? I didn't carefully look at the code that uses this list to see if we have to list everything or can list just the ones we recommend, and erred on the safer side (unlike the one for completion where I omitted bc3 as deprecated). We should drop bc3 here. This is the list of candidates that is used when no configuration exists. Listing bc twice here implies that we might try that candidate twice. Otherwise, this patch looks like the right way to go ~ it makes bc available and keeps compatibility for bc3. If we wanted to phase out bc3 for Git 3.0 we could start warning inside of the bc3 scriptlet, but I don't see any reason to do so a.t.m. Thanks, -- David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Copy mergetool bc3 as bc4
David Aguilar dav...@gmail.com writes: Otherwise, this patch looks like the right way to go ~ it makes bc available and keeps compatibility for bc3. OK, thanks. Here is what I'll queue. -- 8 -- From: Junio C Hamano gits...@pobox.com Date: Mon, 20 Oct 2014 15:49:36 -0700 Subject: [PATCH] mergetool: rename bc3 to bc Beyond Compare version 4 works the same way as version 3, so rename the existing bc3 adaptor to just bc, while keeping bc3 as a backward compatible wrapper. Noticed-by: Olivier Croquette ocroque...@free.fr Helped-by: David Aguilar dav...@gmail.com Signed-off-by: Junio C Hamano gits...@pobox.com --- contrib/completion/git-completion.bash | 2 +- git-mergetool--lib.sh | 2 +- mergetools/bc | 25 + mergetools/bc3 | 26 +- 4 files changed, 28 insertions(+), 27 deletions(-) create mode 100644 mergetools/bc diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 06bf262..8a19b3e 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1207,7 +1207,7 @@ _git_diff () } __git_mergetools_common=diffuse diffmerge ecmerge emerge kdiff3 meld opendiff - tkdiff vimdiff gvimdiff xxdiff araxis p4merge bc3 codecompare + tkdiff vimdiff gvimdiff xxdiff araxis p4merge bc codecompare _git_difftool () diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index c45a020..a40d3df 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -250,7 +250,7 @@ list_merge_tool_candidates () { tools=opendiff kdiff3 tkdiff xxdiff meld $tools fi tools=$tools gvimdiff diffuse diffmerge ecmerge - tools=$tools p4merge araxis bc3 codecompare + tools=$tools p4merge araxis bc codecompare fi case ${VISUAL:-$EDITOR} in *vim*) diff --git a/mergetools/bc b/mergetools/bc new file mode 100644 index 000..b6319d2 --- /dev/null +++ b/mergetools/bc @@ -0,0 +1,25 @@ +diff_cmd () { + $merge_tool_path $LOCAL $REMOTE +} + +merge_cmd () { + touch $BACKUP + if $base_present + then + $merge_tool_path $LOCAL $REMOTE $BASE \ + -mergeoutput=$MERGED + else + $merge_tool_path $LOCAL $REMOTE \ + -mergeoutput=$MERGED + fi + check_unchanged +} + +translate_merge_tool_path() { + if type bcomp /dev/null 2/dev/null + then + echo bcomp + else + echo bcompare + fi +} diff --git a/mergetools/bc3 b/mergetools/bc3 index b6319d2..5d8dd48 100644 --- a/mergetools/bc3 +++ b/mergetools/bc3 @@ -1,25 +1 @@ -diff_cmd () { - $merge_tool_path $LOCAL $REMOTE -} - -merge_cmd () { - touch $BACKUP - if $base_present - then - $merge_tool_path $LOCAL $REMOTE $BASE \ - -mergeoutput=$MERGED - else - $merge_tool_path $LOCAL $REMOTE \ - -mergeoutput=$MERGED - fi - check_unchanged -} - -translate_merge_tool_path() { - if type bcomp /dev/null 2/dev/null - then - echo bcomp - else - echo bcompare - fi -} +. $MERGE_TOOLS_DIR/bc -- 2.1.2-583-g325e495 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Copy mergetool bc3 as bc4
Olivier Croquette ocroque...@free.fr writes: Beyond compare 4 is out since september 2014. The CLI interface doesn't seem to have changed compared to the version 3. Hmph, if this is identical to mergetools/bc3, why is the patch even needed? Do we auto-detect something and try to use bc4 which does not exist and fail, and we must supply a copy as bc4 to prevent it? It may feel somewhat strange to have to say mergetool --tool=bc3 when you know what you have is version 4 and not version 3, but in that case, I wonder if there are reasons why calling both versions just bc is a bad idea. And assuming that version 5 and later would keep the same interface, we will not have to worry about that I have version 7 why do I have to say 3? if we can go that route. Perhaps version 2 and before are unusable as a mergetool backend or something? Signed-off-by: Olivier Croquette ocroque...@free.fr --- mergetools/bc4 | 25 + 1 files changed, 25 insertions(+), 0 deletions(-) create mode 100644 mergetools/bc4 diff --git a/mergetools/bc4 b/mergetools/bc4 new file mode 100644 index 000..b6319d2 --- /dev/null +++ b/mergetools/bc4 @@ -0,0 +1,25 @@ +diff_cmd () { + $merge_tool_path $LOCAL $REMOTE +} + +merge_cmd () { + touch $BACKUP + if $base_present + then + $merge_tool_path $LOCAL $REMOTE $BASE \ + -mergeoutput=$MERGED + else + $merge_tool_path $LOCAL $REMOTE \ + -mergeoutput=$MERGED + fi + check_unchanged +} + +translate_merge_tool_path() { + if type bcomp /dev/null 2/dev/null + then + echo bcomp + else + echo bcompare + fi +} -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Copy mergetool bc3 as bc4
Junio C Hamano gits...@pobox.com writes: Olivier Croquette ocroque...@free.fr writes: Beyond compare 4 is out since september 2014. The CLI interface doesn't seem to have changed compared to the version 3. Hmph, if this is identical to mergetools/bc3, why is the patch even needed? Do we auto-detect something and try to use bc4 which does not exist and fail, and we must supply a copy as bc4 to prevent it? It may feel somewhat strange to have to say mergetool --tool=bc3 when you know what you have is version 4 and not version 3, but in that case, I wonder if there are reasons why calling both versions just bc is a bad idea. And assuming that version 5 and later would keep the same interface, we will not have to worry about that I have version 7 why do I have to say 3? if we can go that route. Perhaps version 2 and before are unusable as a mergetool backend or something? It seems that ffe6dc08 (mergetool--lib: Add Beyond Compare 3 as a tool, 2011-02-27) is the first mention of Beyond Compare and it only was interested in version 3 and nothing else. Perhaps something like this, so that existing users can still use bc3 and other people can use bc if it bothers them that they have to say 3 when the backend driver works with both 3 and 4? --- contrib/completion/git-completion.bash | 2 +- git-mergetool--lib.sh | 2 +- mergetools/{bc3 = bc} | 0 mergetools/bc3 | 26 +- 4 files changed, 3 insertions(+), 27 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index d548e99..01259cc 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1203,7 +1203,7 @@ _git_diff () } __git_mergetools_common=diffuse diffmerge ecmerge emerge kdiff3 meld opendiff - tkdiff vimdiff gvimdiff xxdiff araxis p4merge bc3 codecompare + tkdiff vimdiff gvimdiff xxdiff araxis p4merge bc codecompare _git_difftool () diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index c45a020..1d8a26d 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -250,7 +250,7 @@ list_merge_tool_candidates () { tools=opendiff kdiff3 tkdiff xxdiff meld $tools fi tools=$tools gvimdiff diffuse diffmerge ecmerge - tools=$tools p4merge araxis bc3 codecompare + tools=$tools p4merge araxis bc bc3 codecompare fi case ${VISUAL:-$EDITOR} in *vim*) diff --git a/mergetools/bc3 b/mergetools/bc similarity index 100% rename from mergetools/bc3 rename to mergetools/bc diff --git a/mergetools/bc3 b/mergetools/bc3 dissimilarity index 100% index b6319d2..5d8dd48 100644 --- a/mergetools/bc3 +++ b/mergetools/bc3 @@ -1,25 +1 @@ -diff_cmd () { - $merge_tool_path $LOCAL $REMOTE -} - -merge_cmd () { - touch $BACKUP - if $base_present - then - $merge_tool_path $LOCAL $REMOTE $BASE \ - -mergeoutput=$MERGED - else - $merge_tool_path $LOCAL $REMOTE \ - -mergeoutput=$MERGED - fi - check_unchanged -} - -translate_merge_tool_path() { - if type bcomp /dev/null 2/dev/null - then - echo bcomp - else - echo bcompare - fi -} +. $MERGE_TOOLS_DIR/bc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Copy mergetool bc3 as bc4
On 20.10.2014 19:32, Junio C Hamano wrote: Beyond compare 4 is out since september 2014. The CLI interface doesn't seem to have changed compared to the version 3. Hmph, if this is identical to mergetools/bc3, why is the patch even needed? Do we auto-detect something and try to use bc4 which does not exist and fail, and we must supply a copy as bc4 to prevent it? The 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. It may feel somewhat strange to have to say mergetool --tool=bc3 when you know what you have is version 4 and not version 3, but in That's exactly the only reason I could think of why it could be nice to have a bc4. that case, I wonder if there are reasons why calling both versions just bc is a bad idea. And assuming that version 5 and later IMHO, 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. Perhaps version 2 and before are unusable as a mergetool backend or something? It seems that ffe6dc08 (mergetool--lib: Add Beyond Compare 3 as a tool, 2011-02-27) is the first mention of Beyond Compare and it only was interested in version 3 and nothing else. Beyond 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. Perhaps something like this, so that existing users can still use bc3 and other people can use bc if it bothers them that they have to say 3 when the backend driver works with both 3 and 4? That indeed sounds like the best approach. --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -250,7 +250,7 @@ list_merge_tool_candidates () { tools=opendiff kdiff3 tkdiff xxdiff meld $tools fi tools=$tools gvimdiff diffuse diffmerge ecmerge - tools=$tools p4merge araxis bc3 codecompare + tools=$tools p4merge araxis bc bc3 codecompare Why keep bc3 here? And shouldn't we update git-gui/lib/mergetool.tcl, too? -- Sebastian Schuberth -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Copy mergetool bc3 as bc4
Sebastian Schuberth sschube...@gmail.com writes: Perhaps something like this, so that existing users can still use bc3 and other people can use bc if it bothers them that they have to say 3 when the backend driver works with both 3 and 4? That indeed sounds like the best approach. --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -250,7 +250,7 @@ list_merge_tool_candidates () { tools=opendiff kdiff3 tkdiff xxdiff meld $tools fi tools=$tools gvimdiff diffuse diffmerge ecmerge -tools=$tools p4merge araxis bc3 codecompare +tools=$tools p4merge araxis bc bc3 codecompare Why keep bc3 here? I didn't carefully look at the code that uses this list to see if we have to list everything or can list just the ones we recommend, and erred on the safer side (unlike the one for completion where I omitted bc3 as deprecated). I'll let mergetools experts decide when rolling the final patch ;-) And shouldn't we update git-gui/lib/mergetool.tcl, too? Yes we should, but git-gui is not in my bailiwick, and shouldn't be done relative to my tree anyway. I'll Cc this message to Pat. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Copy mergetool bc3 as bc4
On 20 Oct 2014, at 19:32, Junio C Hamano gits...@pobox.com wrote: Perhaps something like this, so that existing users can still use bc3 and other people can use bc if it bothers them that they have to say 3 when the backend driver works with both 3 and 4? Thanks for the quick and great feedback. This 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. It’s even future safe, because it doesn’t prevent introducing bc5 if required later on. Olivier -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html