{"thread":{"id":"53640","subject":"RE: [PATCH v2] Enable auto-merge for meld to follow the vim-diff beharior","startedAt":"2020-06-09T03:33:59Z","lastAt":"2020-06-30T15:55:34Z","messageCount":6,"participants":["lin.sun@zoom.us","Đoàn Trần Công Danh","David Aguilar","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"399338","messageId":"311401d63e0e$b1ffe490$15ffadb0$@zoom.us","threadId":"53640","inReplyTo":null,"subject":"RE: [PATCH v2] Enable auto-merge for meld to follow the vim-diff beharior","fromName":"","fromEmail":"lin.sun@zoom.us","sentAt":"2020-06-09T03:33:05Z","receivedAt":"2020-06-09T03:33:59Z","isPatch":true,"sender":{"key":"lin.sun@zoom.us","avatar":null},"body":"Hi Pratyush,\n\nYes, you're totally right, it's my typo, and it's fixed by my last commit [PATCH v2]. \nI tried git merge-tool with meld and found there is \"auto-merge\" option for better experience.\nThe \"auto-merge\" was added since meld 1.7 (Year 2012).\nHere're some documents for it.\nhttps://meldmerge.org/features.html\nhttps://lukas.zapletalovi.com/2012/09/three-way-git-merging-with-meld.html\n\nRegards\nLin\n-----Original Message-----\nFrom: Pratyush Yadav \nSent: Monday, June 8, 2020 17:50\nTo: sunlin via GitGitGadget \nCc: git@vger.kernel.org; sunlin ; lin.sun \nSubject: Re: [PATCH] Enable auto-merge for meld to follow the vim-diff beharior\n\nHi Lin,\n\nI'm not familiar with the code so I'll let someone else comment on that. \nBut...\n\nOn 08/05/20 01:25AM, sunlin via GitGitGadget wrote:\n> From: \"lin.sun\" \n> \n> The mergetool \"meld\" does NOT merge the no-conflict changes, while the \n> mergetool \"vimdiff\" will merge the no-conflict parts and highlight the \n> conflict parts.\n> This patch will make the mergetool \"meld\" similar to \"vimdiff\", \n> auto-merge the no-conflict parts, highlight conflict parts.\n> \n> Signed-off-by: Lin Sun \n\n... your name and email in \"From:\" and \"Signed-off-by:\" should be the same. So either use \"lin.sun\" in both places or use Lin Sun in both.\n\n--\nRegards,\nPratyush Yadav\n\n"},{"id":"400733","messageId":"0c2401d64d14$aeafa680$0c0ef380$@zoom.us","threadId":"53640","inReplyTo":"311401d63e0e$b1ffe490$15ffadb0$@zoom.us","subject":"RE: [PATCH v2] Enable auto-merge for meld to follow the vim-diff beharior","fromName":"","fromEmail":"lin.sun@zoom.us","sentAt":"2020-06-28T06:23:43Z","receivedAt":"2020-06-28T06:23:54Z","isPatch":true,"sender":{"key":"lin.sun@zoom.us","avatar":null},"body":"Hi Pratyush,\n\nCould you help to review and merge this patch? Or do you know who can review and merge this patch please?\n\nRegards\nLin\n-----Original Message-----\nFrom: lin.sun@zoom.us \nSent: Tuesday, June 9, 2020 11:33\nTo: 'Pratyush Yadav' ; 'sunlin via GitGitGadget' \nCc: git@vger.kernel.org; 'sunlin' \nSubject: RE: [PATCH v2] Enable auto-merge for meld to follow the vim-diff beharior\n\nHi Pratyush,\n\nYes, you're totally right, it's my typo, and it's fixed by my last commit [PATCH v2]. \nI tried git merge-tool with meld and found there is \"auto-merge\" option for better experience.\nThe \"auto-merge\" was added since meld 1.7 (Year 2012).\nHere're some documents for it.\nhttps://meldmerge.org/features.html\nhttps://lukas.zapletalovi.com/2012/09/three-way-git-merging-with-meld.html\n\nRegards\nLin\n-----Original Message-----\nFrom: Pratyush Yadav \nSent: Monday, June 8, 2020 17:50\nTo: sunlin via GitGitGadget \nCc: git@vger.kernel.org; sunlin ; lin.sun \nSubject: Re: [PATCH] Enable auto-merge for meld to follow the vim-diff beharior\n\nHi Lin,\n\nI'm not familiar with the code so I'll let someone else comment on that. \nBut...\n\nOn 08/05/20 01:25AM, sunlin via GitGitGadget wrote:\n> From: \"lin.sun\" \n> \n> The mergetool \"meld\" does NOT merge the no-conflict changes, while the \n> mergetool \"vimdiff\" will merge the no-conflict parts and highlight the \n> conflict parts.\n> This patch will make the mergetool \"meld\" similar to \"vimdiff\", \n> auto-merge the no-conflict parts, highlight conflict parts.\n> \n> Signed-off-by: Lin Sun \n\n... your name and email in \"From:\" and \"Signed-off-by:\" should be the same. So either use \"lin.sun\" in both places or use Lin Sun in both.\n\n--\nRegards,\nPratyush Yadav\n\n\n"},{"id":"400737","messageId":"20200628103755.GB26319@danh.dev","threadId":"53640","inReplyTo":"0c2401d64d14$aeafa680$0c0ef380$@zoom.us","subject":"Re: [PATCH v2] Enable auto-merge for meld to follow the vim-diff beharior","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-06-28T10:37:55Z","receivedAt":"2020-06-28T10:37:59Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-06-28 14:23:43+0800, lin.sun@zoom.us wrote:\n> Hi Pratyush,\n> \n> Could you help to review and merge this patch?\n> Or do you know who can review and merge this patch please?\n\nIt seems like only David Aguilar (Cc-ed) works on that file.\n\nLook into b12d04503b (mergetools/meld: make usage of `--output`\nconfigurable and more robust, 2014-10-15), it looks like we need to\ncheck if --auto-merge option is available in meld.\n\nSomeone still live with the ancient tools ;).\nmeld has known --output for a long time but we still have a check for\nit.\n\n-- \nDanh\n"},{"id":"400764","messageId":"bfc401d64df4$cd189e70$6749db50$@zoom.us","threadId":"53640","inReplyTo":"20200628103755.GB26319@danh.dev","subject":"RE: [PATCH v2] Enable auto-merge for meld to follow the vim-diff beharior","fromName":"","fromEmail":"lin.sun@zoom.us","sentAt":"2020-06-29T09:08:02Z","receivedAt":"2020-06-29T20:51:24Z","isPatch":true,"sender":{"key":"lin.sun@zoom.us","avatar":null},"body":"Hi Danh,\n\n> It seems like only David Aguilar (Cc-ed) works on that file.\n>Look into b12d04503b (mergetools/meld: make usage of `--output` configurable and more robust, 2014-10-15), it looks like we need to check if --auto-merge option is available in meld.\n>Someone still live with the ancient tools ;).\n>meld has known --output for a long time but we still have a check for it.\n\nThank you for your hints, I changed the patch for checking the option \"--auto-merge\" in meld and use this option only if it's available.\nThe last patch is appended in attachment, or https://github.com/git/git/commit/3b70fd0bfc4086a08e27c869ff7492d567e8fdc2\n\nLook forward it can be merged this time. \nThanks\n\nRegards\nLin\n\n\nFrom 3b70fd0bfc4086a08e27c869ff7492d567e8fdc2 Mon Sep 17 00:00:00 2001\nFrom: \"lin.sun\" \nDate: Thu, 7 May 2020 07:31:14 +0800\nSubject: [PATCH] Enable auto-merge for meld to follow the vim-diff beharior\n\nThe mergetool \"meld\" does NOT merge the no-conflict changes, while the\nmergetool \"vimdiff\" will merge the no-conflict parts and highlight the\nconflict parts.\nThis patch will make the mergetool \"meld\" similar to \"vimdiff\",\nauto-merge the no-conflict parts, highlight conflict parts.\n\nSigned-off-by: Lin Sun \n---\n mergetools/meld | 32 ++++++++++++++++++++++++++++++--\n 1 file changed, 30 insertions(+), 2 deletions(-)\n\ndiff --git a/mergetools/meld b/mergetools/meld\nindex 7a08470..91b65ff 100644\n--- a/mergetools/meld\n+++ b/mergetools/meld\n@@ -7,13 +7,23 @@ merge_cmd () {\n \tthen\n \t\tcheck_meld_for_output_version\n \tfi\n+\tif test -z \"${meld_has_auto_merge_option:+set}\"\n+\tthen\n+\t\tcheck_meld_for_auto_merge_version\n+\tfi\n+\n+\toption_auto_merge=\n+\tif test \"$meld_has_auto_merge_option\" = true\n+\tthen\n+\t\toption_auto_merge=\"--auto-merge\"\n+\tfi\n \n \tif test \"$meld_has_output_option\" = true\n \tthen\n-\t\t\"$merge_tool_path\" --output=\"$MERGED\" \\\n+\t\t\"$merge_tool_path\" $option_auto_merge --output=\"$MERGED\" \\\n \t\t\t\"$LOCAL\" \"$BASE\" \"$REMOTE\"\n \telse\n-\t\t\"$merge_tool_path\" \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n+\t\t\"$merge_tool_path\" $option_auto_merge \"$LOCAL\" \"$MERGED\" \"$REMOTE\"\n \tfi\n }\n \n@@ -34,3 +44,21 @@ check_meld_for_output_version () {\n \t\tmeld_has_output_option=false\n \tfi\n }\n+\n+# Check whether we should use 'meld --auto-merge ...'\n+check_meld_for_auto_merge_version () {\n+\tmeld_path=\"$(git config mergetool.meld.path)\"\n+\tmeld_path=\"${meld_path:-meld}\"\n+\n+\tif meld_has_auto_merge_option=$(git config --bool mergetool.meld.hasAutoMerge)\n+\tthen\n+\t\t: use configured value\n+\telif \"$meld_path\" --help 2>&1 |\n+\t\tgrep -e '--auto-merge' -e '\\[OPTION\\.\\.\\.\\]' >/dev/null\n+\tthen\n+\t\t: old ones mention --auto-merge and new ones just say OPTION...\n+\t\tmeld_has_auto_merge_option=true\n+\telse\n+\t\tmeld_has_auto_merge_option=false\n+\tfi\n+}\n-- \n2.2.0\n\n"},{"id":"400793","messageId":"20200630064800.GB1962986@gmail.com","threadId":"53640","inReplyTo":"bfc401d64df4$cd189e70$6749db50$@zoom.us","subject":"Re: [PATCH v2] Enable auto-merge for meld to follow the vim-diff beharior","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2020-06-30T06:48:00Z","receivedAt":"2020-06-30T06:48:05Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Mon, Jun 29, 2020 at 05:08:02PM +0800, lin.sun@zoom.us wrote:\n> Hi Danh,\n> \n> > It seems like only David Aguilar (Cc-ed) works on that file.\n> >Look into b12d04503b (mergetools/meld: make usage of `--output` configurable and more robust, 2014-10-15), it looks like we need to check if --auto-merge option is available in meld.\n> >Someone still live with the ancient tools ;).\n> >meld has known --output for a long time but we still have a check for it.\n> \n> Thank you for your hints, I changed the patch for checking the option \"--auto-merge\" in meld and use this option only if it's available.\n> The last patch is appended in attachment, or https://github.com/git/git/commit/3b70fd0bfc4086a08e27c869ff7492d567e8fdc2\n> \n> Look forward it can be merged this time. \n> Thanks\n> \n> Regards\n> Lin\n\nHello, thank you for making this change and keeping the checks around\nfor the older versions.\n\nOne comment on the patch -- it looks like it duplicates the version\ncheck logic. Would it be possible to coalesce the version check so that\nwe only exec meld once for the two version checks?\n\nThe changes look good nonetheless, and perhaps combining the checks\nwould make it too complex, so this looks good to me.\n\nCan you please submit the patch to the list here?\nOnce you do, please feel free to add my sign-off:\n\nSigned-off-by: David Aguilar \n\ncheers,\n-- \nDavid\n"},{"id":"400833","messageId":"xmqqwo3oeefm.fsf@gitster.c.googlers.com","threadId":"53640","inReplyTo":"20200630064800.GB1962986@gmail.com","subject":"Re: [PATCH v2] Enable auto-merge for meld to follow the vim-diff beharior","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-06-30T15:55:25Z","receivedAt":"2020-06-30T15:55:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Aguilar writes:\n\n> Can you please submit the patch to the list here?\n> Once you do, please feel free to add my sign-off:\n>\n> Signed-off-by: David Aguilar \n\nThanks, but you didn't write a single line of new code in the patch,\nand you are not relaying Sun Lin's patch on the author's behalf, so\nSob is probably not what you meant---it would either be Reviewed-by\n(I've gone through the patch with fine toothed comb, I can answer\nany questions about the change as well as the author could, and I\ncan stand behind it with confidence) or Acked-by (I've seen the\npatch, didn't see anything outrageously wrong in it, and agree with\nits general direction---I appreciate the author tacking the issue).\n"}]}