{"thread":{"id":"19154","subject":"[PATCH] mergetool--lib: specialize diff options for emerge and ecmerge","startedAt":"2009-05-02T08:57:21Z","lastAt":"2009-05-03T20:34:27Z","messageCount":7,"participants":["David Aguilar","Markus Heidelberg","Marcin Zalewski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"112868","messageId":"1241254641-54338-1-git-send-email-davvid@gmail.com","threadId":"19154","inReplyTo":null,"subject":"[PATCH] mergetool--lib: specialize diff options for emerge and ecmerge","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-05-02T08:57:21Z","receivedAt":"2009-05-02T08:57:21Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"The ecmerge documentation mentions the following form:\n\n\tecmerge --mode=diff2 $1 $2\n\nSince git-difftool is about diffing, we should use that\ninstead of --mode=merge2. Likewise, this drops the\n$MERGED argument to emerge, as discussed on the git list\n($gmane/117930).\n\nSigned-off-by: David Aguilar \n---\n\nI tested the emacs (emerge) bit, but I'm not an emacs\nuser and I didn't really see any difference with or\nwithout the patch. Dropping $MERGED seems like the\nright thing to do nonetheless.\n\nIn emerge/emacs mode, we still end up seeing the\nmerge pane. My emacs-fu is not sophisticated\nenough to know how to inhibit the merge pane\n(if that's even something we'd want to do).\n\n\nRegarding ecmerge: I found the --mode=diff2\nflag by reading their documenation:\n\nhttp://www.elliecomputing.com/OnlineDoc/ecmerge_EN/52335623.asp\n\nI don't have ecmerge installed at all, so I'm just\ngoing by the book on this one. It *looks* correct,\nand probably is, but let it be known that I\nhaven't tested the ecmerge snippet myself.\n\n git-mergetool--lib.sh | 6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\nindex a16a279..8b5e6a8 100644\n--- a/git-mergetool--lib.sh\n+++ b/git-mergetool--lib.sh\n@@ -228,8 +228,8 @@ run_merge_tool () {\n \t\t\tfi\n \t\t\tcheck_unchanged\n \t\telse\n-\t\t\t\"$merge_tool_path\" \"$LOCAL\" \"$REMOTE\" \\\n-\t\t\t\t--default --mode=merge2 --to=\"$MERGED\"\n+\t\t\t\"$merge_tool_path\" --default --mode=diff2 \\\n+\t\t\t\t\"$LOCAL\" \"$REMOTE\"\n \t\tfi\n \t\t;;\n \temerge)\n@@ -248,7 +248,7 @@ run_merge_tool () {\n \t\t\tstatus=$?\n \t\telse\n \t\t\t\"$merge_tool_path\" -f emerge-files-command \\\n-\t\t\t\t\"$LOCAL\" \"$REMOTE\" \"$(basename \"$MERGED\")\"\n+\t\t\t\t\"$LOCAL\" \"$REMOTE\"\n \t\tfi\n \t\t;;\n \ttortoisemerge)\n-- \n1.6.3.rc3.40.g75b44\n"},{"id":"112885","messageId":"20090502194636.GA599@gmail.com","threadId":"19154","inReplyTo":"1241254641-54338-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH] mergetool--lib: specialize diff options for emerge and ecmerge","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-05-02T19:46:37Z","receivedAt":"2009-05-02T19:46:37Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On 0, David Aguilar wrote:\n> \n> Regarding ecmerge: I found the --mode=diff2\n> flag by reading their documenation:\n> \n> http://www.elliecomputing.com/OnlineDoc/ecmerge_EN/52335623.asp\n> \n> I don't have ecmerge installed at all, so I'm just\n> going by the book on this one. It *looks* correct,\n> and probably is, but let it be known that I\n> haven't tested the ecmerge snippet myself.\n\n\nI installed ecmerge on a mac today and gave this a try.\necmerge is indeed better with this patch.\n\nAfter configuring the path it all \"just works\":\n\n$ git config --global mergetool.ecmerge.path \\\n\t/Applications/ECMerge.app/Contents/MacOS/guimerge\n\nWe now get a simple side-by-side diff without the\nmerge pane at the bottom of the screen. Nice.\n\n\nIf an emacs user could comment on the emerge snippet\nbelow (or perhaps suggest a better one ;)) then that\nwould make me happy. As is, I have tested the\nemacs emerge snippet and it works, but I'm not\nsure if that's enough to resolve the issue reported\nby Marcin. Marcin?\n\nHere's the original thread:\nhttp://article.gmane.org/gmane.comp.version-control.git/117930\n\n\n> diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh\n> index a16a279..8b5e6a8 100644\n> --- a/git-mergetool--lib.sh\n> +++ b/git-mergetool--lib.sh\n> @@ -248,7 +248,7 @@ run_merge_tool () {\n> \t\t\tstatus=$?\n> \t\telse\n> \t\t\t\"$merge_tool_path\" -f emerge-files-command \\\n> -\t\t\t\t\"$LOCAL\" \"$REMOTE\" \"$(basename \"$MERGED\")\"\n> +\t\t\t\t\"$LOCAL\" \"$REMOTE\"\n> \t\tfi\n> \t\t;;\n\n-- \n\t\tDavid\n"},{"id":"112906","messageId":"200905030827.56932.markus.heidelberg@web.de","threadId":"19154","inReplyTo":"20090502194636.GA599@gmail.com","subject":"Re: [PATCH] mergetool--lib: specialize diff options for emerge and ecmerge","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-05-03T06:27:56Z","receivedAt":"2009-05-03T06:27:56Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"David Aguilar, 02.05.2009:\n> I installed ecmerge on a mac today and gave this a try.\n> ecmerge is indeed better with this patch.\n> \n> After configuring the path it all \"just works\":\n> \n> $ git config --global mergetool.ecmerge.path \\\n> \t/Applications/ECMerge.app/Contents/MacOS/guimerge\n\nWould it make sense to set merge_tool_path to guimerge by default then?\n\nMarkus\n"},{"id":"112925","messageId":"20090503182137.GC50640@gmail.com","threadId":"19154","inReplyTo":"200905030827.56932.markus.heidelberg@web.de","subject":"Re: [PATCH] mergetool--lib: specialize diff options for emerge and ecmerge","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-05-03T18:21:38Z","receivedAt":"2009-05-03T18:21:38Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Markus Heidelberg wrote:\n> David Aguilar, 02.05.2009:\n> > I installed ecmerge on a mac today and gave this a try.\n> > ecmerge is indeed better with this patch.\n> > \n> > After configuring the path it all \"just works\":\n> > \n> > $ git config --global mergetool.ecmerge.path \\\n> > \t/Applications/ECMerge.app/Contents/MacOS/guimerge\n> \n> Would it make sense to set merge_tool_path to guimerge by default then?\n> \n> Markus\n\nOn Linux, ecmerge is \"ecmerge\".\nMacs are weird. Windows--even worse.\n\nWe could test $(uname) = \"Darwin\" and do the user-friendly thing by default,\nbut that might not be a good idea. The user-friendly thing is actually\n\"/Applications/...lots.of.stuff.../guimerge\", and that's a lot more\nplatform-specific than just \"guimerge\".\n\nThe usability fairy says we should be nice to users and turn\ntranslate_merge_tool_path() into a massive platform-specific \nhack. The lazy person in me would rather list the tweaks\non the git wiki and silently reward linux users since\nthe defaults work fine there as-is.\n\nWhat do you think?\n\n-- \n\t\tDavid\n"},{"id":"112926","messageId":"7c0fdf4f0905031147o47a7d2b1wbaa80df9058afeb6@mail.gmail.com","threadId":"19154","inReplyTo":"20090502194636.GA599@gmail.com","subject":"Re: [PATCH] mergetool--lib: specialize diff options for emerge and ecmerge","fromName":"Marcin Zalewski","fromEmail":"marcin.zalewski@gmail.com","sentAt":"2009-05-03T18:47:38Z","receivedAt":"2009-05-03T18:47:38Z","isPatch":true,"sender":{"key":"marcin.zalewski@gmail.com","avatar":"https://gravatar.com/avatar/df5a1c06f2fd2fc729fd0e540c4802b598695ef40d9fb0ed11bfb4d3e17a2c04?d=mp&s=160"},"body":"David,\n\nThanks for your patches.\n\n> If an emacs user could comment on the emerge snippet\n> below (or perhaps suggest a better one ;)) then that\n> would make me happy.  As is, I have tested the\n> emacs emerge snippet and it works, but I'm not\n> sure if that's enough to resolve the issue reported\n> by Marcin.  Marcin?\n>\n> Here's the original thread:\n> http://article.gmane.org/gmane.comp.version-control.git/117930\n\nAs far as I am concerned, this patch works well. Without the third\nargument, ediff will not complain anymore that it is \"too dangerous to\nmerge versions of a file visited by another buffer,\" which was my\noriginal problem.\n\n-Marcin\n"},{"id":"112928","messageId":"200905032112.31156.markus.heidelberg@web.de","threadId":"19154","inReplyTo":"20090503182137.GC50640@gmail.com","subject":"Re: [PATCH] mergetool--lib: specialize diff options for emerge and ecmerge","fromName":"Markus Heidelberg","fromEmail":"markus.heidelberg@web.de","sentAt":"2009-05-03T19:12:30Z","receivedAt":"2009-05-03T19:12:30Z","isPatch":true,"sender":{"key":"markus.heidelberg@web.de","avatar":"https://avatars.githubusercontent.com/u/6334512?v=4"},"body":"David Aguilar, 03.05.2009:\n> Markus Heidelberg wrote:\n> > David Aguilar, 02.05.2009:\n> > > I installed ecmerge on a mac today and gave this a try.\n> > > ecmerge is indeed better with this patch.\n> > > \n> > > After configuring the path it all \"just works\":\n> > > \n> > > $ git config --global mergetool.ecmerge.path \\\n> > > \t/Applications/ECMerge.app/Contents/MacOS/guimerge\n> > \n> > Would it make sense to set merge_tool_path to guimerge by default then?\n> > \n> > Markus\n> \n> On Linux, ecmerge is \"ecmerge\".\n> Macs are weird. Windows--even worse.\n\nAnd the ecmerge developers are even more worse or is there a reason to\nhave different names for the binaries?\n\n> We could test $(uname) = \"Darwin\" and do the user-friendly thing by default,\n> but that might not be a good idea.\n\nOr use \"type\" there already to test whether guimerge exists, else\ndefault to ecmerge.\n\n> The user-friendly thing is actually\n> \"/Applications/...lots.of.stuff.../guimerge\", and that's a lot more\n> platform-specific than just \"guimerge\".\n\nWhy is the whole path more user friendly, because of guimerge not being\nin PATH? Is the /Applications/... directory always the same for ecmerge\nfor each Mac user? But we don't care in other diff/merge tools about the\nexact location and I think we shouldn't begin it here.\n\n> The usability fairy says we should be nice to users and turn\n> translate_merge_tool_path() into a massive platform-specific \n> hack. The lazy person in me would rather list the tweaks\n> on the git wiki and silently reward linux users since\n> the defaults work fine there as-is.\n> \n> What do you think?\n\nNot sure, leaving it as is is still an option.\n"},{"id":"112934","messageId":"20090503203427.GA22440@gmail.com","threadId":"19154","inReplyTo":"200905032112.31156.markus.heidelberg@web.de","subject":"Re: [PATCH] mergetool--lib: specialize diff options for emerge and ecmerge","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2009-05-03T20:34:27Z","receivedAt":"2009-05-03T20:34:27Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Markus Heidelberg wrote:\n> > \n> > On Linux, ecmerge is \"ecmerge\".\n> > Macs are weird. Windows--even worse.\n> \n> And the ecmerge developers are even more worse or is there a reason to\n> have different names for the binaries?\n\nBut of course :)\n\n\n> > The user-friendly thing is actually\n> > \"/Applications/...lots.of.stuff.../guimerge\", and that's a lot more\n> > platform-specific than just \"guimerge\".\n> \n> Why is the whole path more user friendly, because of guimerge not being\n> in PATH? Is the /Applications/... directory always the same for ecmerge\n\nYup.\n\n\n> for each Mac user? But we don't care in other diff/merge tools about the\n> exact location and I think we shouldn't begin it here.\n\nDitto.\n\n\n> > What do you think?\n> \n> Not sure, leaving it as is is still an option.\n\nI agree. So the original patch for ecmerge + emerge should be\ngood then. I still need to reroll the araxis patch after my bike\nride later.\n\n-- \n\n\tDavid\n"}]}