{"thread":{"id":"19973","subject":"[PATCH] Always output a trailing space conflicted merge markers.","startedAt":"2009-06-30T00:41:01Z","lastAt":"2009-07-01T03:42:20Z","messageCount":4,"participants":["Nelson Elhage","Junio C Hamano","Nanako Shiraishi"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"117199","messageId":"1246322461-24742-1-git-send-email-nelhage@mit.edu","threadId":"19973","inReplyTo":null,"subject":"[PATCH] Always output a trailing space conflicted merge markers.","fromName":"Nelson Elhage","fromEmail":"nelhage@mit.edu","sentAt":"2009-06-30T00:41:01Z","receivedAt":"2009-06-30T00:41:01Z","isPatch":true,"sender":{"key":"nelhage@mit.edu","avatar":"https://avatars.githubusercontent.com/u/16725?v=4"},"body":"Some tools, such as emacs' smerge-mode, except that diff3-style merge\nconflict markers always include a trailing space (and optional\ndescription) after the marker, and so fail to correctly detect\ngit's (notably the '|||||||' base marker, for which git never outputs a\ntrailing space).\n\n(See http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=3553\nfor the emacs issue)\n\nSigned-off-by: Nelson Elhage \n---\n xdiff/xmerge.c | 11 ++++++++---\n 1 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/xdiff/xmerge.c b/xdiff/xmerge.c\nindex 1cb65a9..a4baba8 100644\n--- a/xdiff/xmerge.c\n+++ b/xdiff/xmerge.c\n@@ -148,8 +148,8 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,\n \t\t\t xdmerge_t *m, char *dest)\n {\n \tconst int marker_size = 7;\n-\tint marker1_size = (name1 ? strlen(name1) + 1 : 0);\n-\tint marker2_size = (name2 ? strlen(name2) + 1 : 0);\n+\tint marker1_size = (name1 ? strlen(name1) + 1 : 1);\n+\tint marker2_size = (name2 ? strlen(name2) + 1 : 1);\n \tint j;\n \n \t/* Before conflicting part */\n@@ -165,6 +165,8 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,\n \t\t\tdest[size] = ' ';\n \t\t\tmemcpy(dest + size + 1, name1, marker1_size - 1);\n \t\t\tsize += marker1_size;\n+\t\t} else {\n+\t\t\tdest[size++] = ' ';\n \t\t}\n \t\tdest[size++] = '\\n';\n \t}\n@@ -176,10 +178,11 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,\n \tif (style == XDL_MERGE_DIFF3) {\n \t\t/* Shared preimage */\n \t\tif (!dest) {\n-\t\t\tsize += marker_size + 1;\n+\t\t\tsize += marker_size + 2;\n \t\t} else {\n \t\t\tfor (j = 0; j < marker_size; j++)\n \t\t\t\tdest[size++] = '|';\n+\t\t\tdest[size++] = ' ';\n \t\t\tdest[size++] = '\\n';\n \t\t}\n \t\tsize += xdl_orig_copy(xe1, m->i0, m->chg0, 1,\n@@ -206,6 +209,8 @@ static int fill_conflict_hunk(xdfenv_t *xe1, const char *name1,\n \t\t\tdest[size] = ' ';\n \t\t\tmemcpy(dest + size + 1, name2, marker2_size - 1);\n \t\t\tsize += marker2_size;\n+\t\t} else {\n+\t\t\tdest[size++] = ' ';\n \t\t}\n \t\tdest[size++] = '\\n';\n \t}\n-- \n1.6.3.3\n"},{"id":"117268","messageId":"7v1vp15ppi.fsf@alter.siamese.dyndns.org","threadId":"19973","inReplyTo":"1246322461-24742-1-git-send-email-nelhage@mit.edu","subject":"Re: [PATCH] Always output a trailing space conflicted merge markers.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-30T22:20:09Z","receivedAt":"2009-06-30T22:20:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nelson Elhage writes:\n\n> Some tools, such as emacs' smerge-mode, except that diff3-style merge\n> conflict markers always include a trailing space (and optional\n> description) after the marker, and so fail to correctly detect\n> git's (notably the '|||||||' base marker, for which git never outputs a\n> trailing space).\n>\n> (See http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=3553\n> for the emacs issue)\n>\n> Signed-off-by: Nelson Elhage \n> ---\n> xdiff/xmerge.c | 11 ++++++++---\n> 1 files changed, 8 insertions(+), 3 deletions(-)\n\nThanks, but I think you also need to update the test scripts.\n\nDon't you want some new tests to protect this from getting broken\nin the future by other people yourself, too?\n"},{"id":"117281","messageId":"7vzlbpxitn.fsf@alter.siamese.dyndns.org","threadId":"19973","inReplyTo":"7v1vp15ppi.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Always output a trailing space conflicted merge markers.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-07-01T02:01:24Z","receivedAt":"2009-07-01T02:01:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Nelson Elhage writes:\n>\n>> Some tools, such as emacs' smerge-mode, except that diff3-style merge\n>> conflict markers always include a trailing space (and optional\n>> description) after the marker, and so fail to correctly detect\n>> git's (notably the '|||||||' base marker, for which git never outputs a\n>> trailing space).\n>>\n>> (See http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=3553\n>> for the emacs issue)\n>>\n>> Signed-off-by: Nelson Elhage \n>> ---\n>> xdiff/xmerge.c | 11 ++++++++---\n>> 1 files changed, 8 insertions(+), 3 deletions(-)\n>\n> Thanks, but I think you also need to update the test scripts.\n>\n> Don't you want some new tests to protect this from getting broken\n> in the future by other people yourself, too?\n\nI actually changed my mind. The plain \"diff3\" (from GNU diffutils 2.8.1)\ndoes not seem to give an trailing space after ||||||| separator. It\nfeels very wrong to change the output format to work around a bug in\nsmerge-mode (which I never use, by the way), if we are not the only\noffender. With this change, we may be breaking something else that relied\non the \"diff3\" output format that does not have such a trailing space.\n\nI'll drop this patch.\n"},{"id":"117284","messageId":"20090701124220.6117@nanako3.lavabit.com","threadId":"19973","inReplyTo":"7v1vp15ppi.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Always output a trailing space conflicted merge markers.","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-07-01T03:42:20Z","receivedAt":"2009-07-01T03:42:20Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano :\n\n> Nelson Elhage writes:\n>\n>> Some tools, such as emacs' smerge-mode, except that diff3-style merge\n>> conflict markers always include a trailing space (and optional\n>> description) after the marker, and so fail to correctly detect\n>> git's (notably the '|||||||' base marker, for which git never outputs a\n>> trailing space).\n>>\n>> (See http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=3553\n>> for the emacs issue)\n>>\n>> Signed-off-by: Nelson Elhage \n>> ---\n>> xdiff/xmerge.c | 11 ++++++++---\n>> 1 files changed, 8 insertions(+), 3 deletions(-)\n>\n> Thanks, but I think you also need to update the test scripts.\n>\n> Don't you want some new tests to protect this from getting broken\n> in the future by other people yourself, too?\n\nI don't know what future breakages Nelson wants to protect his code from, but this change breaks the test for union merge in t6026-merge-attr.sh.\n\nIt isn't that the test script expects no space after the conflict marker, but with Nelson's patch, the union merge no longer works.\n\nI request this patch to be dropped.\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"}]}