{"thread":{"id":"61802","subject":"[PATCH] add-patch: handle splitting hunks with diff.suppressBlankEmpty","startedAt":"2024-07-18T14:23:43Z","lastAt":"2024-07-20T16:02:08Z","messageCount":6,"participants":["Phillip Wood via GitGitGadget","Junio C Hamano","Phillip Wood"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"498921","messageId":"pull.1763.git.1721312619822.gitgitgadget@gmail.com","threadId":"61802","inReplyTo":null,"subject":"[PATCH] add-patch: handle splitting hunks with diff.suppressBlankEmpty","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-07-18T14:23:39Z","receivedAt":"2024-07-18T14:23:43Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nWhen \"add -p\" parses diffs, it looks for context lines starting with a\nsingle space. But when diff.suppressBlankEmpty is in effect, an empty\ncontext line will omit the space, giving us a true empty line. This\nconfuses the parser, which is unable to split based on such a line.\n\nIt's tempting to say that we should just make sure that we generate a\ndiff without that option.  However, although we do not parse hunks that\nthe user has manually edited with parse_diff() we do allow the user\nto split such hunks. As POSIX calls the decision of whether to print the\nspace here \"implementation-defined\" we need to handle edited hunks where\nempty context lines omit the space.\n\nSo let's handle both cases: a context line either starts with a space or\nconsists of a totally empty line by normalizing the first character to a\nspace when we parse them. Normalizing the first character rather than\nchanging the code to check for a space or newline will hopefully future\nproof against introducing similar bugs if the code is changed.\n\nReported-by: Ilya Tumaykin <itumaykin@gmail.com>\nHelped-by: Jeff King <peff@peff.net>\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n    add-patch: handle splitting hunks with diff.suppressBlankEmpty\n    \n    This is an alternative to jk/add-patch-with-suppress-blank-empty which\n    was recently discarded from next. I hope that normalizing the context\n    marker will simplify any future changes to the code.\n    \n    While I was writing this I realized that we should be recalculating\n    hunk->splittable_into when we re-count the hunk header of it is edited\n    but I've left to for a future series.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1763%2Fphillipwood%2Fadd-p-suppress-blank-empty-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1763/phillipwood/add-p-suppress-blank-empty-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1763\n\n add-patch.c                | 17 ++++++++++++-----\n t/t3701-add-interactive.sh | 11 +++++++++++\n 2 files changed, 23 insertions(+), 5 deletions(-)\n\ndiff --git a/add-patch.c b/add-patch.c\nindex d8ea05ff108..13b2607f544 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -400,6 +400,12 @@ static void complete_file(char marker, struct hunk *hunk)\n \t\thunk->splittable_into++;\n }\n \n+/* Empty context lines may omit the leading ' ' */\n+static int normalize_marker(char *p)\n+{\n+\treturn p[0] == '\\n' || (p[0] == '\\r' && p[1] == '\\n') ? ' ' : p[0];\n+}\n+\n static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n {\n \tstruct strvec args = STRVEC_INIT;\n@@ -485,6 +491,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \twhile (p != pend) {\n \t\tchar *eol = memchr(p, '\\n', pend - p);\n \t\tconst char *deleted = NULL, *mode_change = NULL;\n+\t\tchar ch = normalize_marker(p);\n \n \t\tif (!eol)\n \t\t\teol = pend;\n@@ -532,7 +539,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\t\t * Start counting into how many hunks this one can be\n \t\t\t * split\n \t\t\t */\n-\t\t\tmarker = *p;\n+\t\t\tmarker = ch;\n \t\t} else if (hunk == &file_diff->head &&\n \t\t\t   starts_with(p, \"new file\")) {\n \t\t\tfile_diff->added = 1;\n@@ -586,10 +593,10 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\t\t    (int)(eol - (plain->buf + file_diff->head.start)),\n \t\t\t    plain->buf + file_diff->head.start);\n \n-\t\tif ((marker == '-' || marker == '+') && *p == ' ')\n+\t\tif ((marker == '-' || marker == '+') && ch == ' ')\n \t\t\thunk->splittable_into++;\n-\t\tif (marker && *p != '\\\\')\n-\t\t\tmarker = *p;\n+\t\tif (marker && ch != '\\\\')\n+\t\t\tmarker = ch;\n \n \t\tp = eol == pend ? pend : eol + 1;\n \t\thunk->end = p - plain->buf;\n@@ -953,7 +960,7 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff,\n \tcontext_line_count = 0;\n \n \twhile (splittable_into > 1) {\n-\t\tch = s->plain.buf[current];\n+\t\tch = normalize_marker(s->plain.buf + current);\n \n \t\tif (!ch)\n \t\t\tBUG(\"buffer overrun while splitting hunks\");\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 5d78868ac16..351dd2b4332 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -1164,4 +1164,15 @@ test_expect_success 'reset -p with unmerged files' '\n \ttest_must_be_empty staged\n '\n \n+test_expect_success 'hunk splitting works with diff.suppressBlankEmpty' '\n+\ttest_config diff.suppressBlankEmpty true &&\n+\ttest_write_lines a b c \"\" d e f >file &&\n+\tgit add file &&\n+\ttest_write_lines p q r \"\" s t u >file &&\n+\ttest_write_lines s n y q | git add -p &&\n+\tgit cat-file blob :file >actual &&\n+\ttest_write_lines a b c \"\" s t u >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n\nbase-commit: 790a17fb19d6eadd16c52e5d284a5c6921744766\n-- \ngitgitgadget\n"},{"id":"498930","messageId":"xmqqikx2wtn4.fsf@gitster.g","threadId":"61802","inReplyTo":"pull.1763.git.1721312619822.gitgitgadget@gmail.com","subject":"Re: [PATCH] add-patch: handle splitting hunks with diff.suppressBlankEmpty","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-18T16:29:35Z","receivedAt":"2024-07-18T16:29:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Phillip Wood via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>\n> When \"add -p\" parses diffs, it looks for context lines starting with a\n> single space. But when diff.suppressBlankEmpty is in effect, an empty\n> context line will omit the space, giving us a true empty line. This\n> confuses the parser, which is unable to split based on such a line.\n>\n> It's tempting to say that we should just make sure that we generate a\n> diff without that option.  However, although we do not parse hunks that\n> the user has manually edited with parse_diff() we do allow the user\n> to split such hunks. As POSIX calls the decision of whether to print the\n> space here \"implementation-defined\" we need to handle edited hunks where\n> empty context lines omit the space.\n>\n> So let's handle both cases: a context line either starts with a space or\n> consists of a totally empty line by normalizing the first character to a\n> space when we parse them. Normalizing the first character rather than\n> changing the code to check for a space or newline will hopefully future\n> proof against introducing similar bugs if the code is changed.\n>\n> Reported-by: Ilya Tumaykin <itumaykin@gmail.com>\n> Helped-by: Jeff King <peff@peff.net>\n> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n> ---\n\nWell written.  Thanks for a pleasant read.\n\n> @@ -953,7 +960,7 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff,\n>  \tcontext_line_count = 0;\n>  \n>  \twhile (splittable_into > 1) {\n> -\t\tch = s->plain.buf[current];\n> +\t\tch = normalize_marker(s->plain.buf + current);\n\nI wondered if &s->plain.buf[current] is easier to grok, but what's\nwritten already is good enough ;-)\n\nThere is another explicit mention of ' ' in merge_hunks().  Unless\nwe are normalizing the buffer here (which I do not think we do),\nwouldn't we have to make sure that the code over there also knows\nthat an empty line in a patch is an unmodified empty line?\n\n                if (plain[overlap_end] != ' ')\n                        return error(_(\"expected context line \"\n                                       \"#%d in\\n%.*s\"),\n\n"},{"id":"498969","messageId":"6c7dfa30-6d4e-4b87-a25a-c1f85c83a6b1@gmail.com","threadId":"61802","inReplyTo":"xmqqikx2wtn4.fsf@gitster.g","subject":"Re: [PATCH] add-patch: handle splitting hunks with diff.suppressBlankEmpty","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2024-07-19T15:17:50Z","receivedAt":"2024-07-19T15:17:55Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 18/07/2024 17:29, Junio C Hamano wrote:\n> \"Phillip Wood via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n> \n>> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>>\n>> When \"add -p\" parses diffs, it looks for context lines starting with a\n>> single space. But when diff.suppressBlankEmpty is in effect, an empty\n>> context line will omit the space, giving us a true empty line. This\n>> confuses the parser, which is unable to split based on such a line.\n>>\n>> It's tempting to say that we should just make sure that we generate a\n>> diff without that option.  However, although we do not parse hunks that\n>> the user has manually edited with parse_diff() we do allow the user\n>> to split such hunks. As POSIX calls the decision of whether to print the\n>> space here \"implementation-defined\" we need to handle edited hunks where\n>> empty context lines omit the space.\n>>\n>> So let's handle both cases: a context line either starts with a space or\n>> consists of a totally empty line by normalizing the first character to a\n>> space when we parse them. Normalizing the first character rather than\n>> changing the code to check for a space or newline will hopefully future\n>> proof against introducing similar bugs if the code is changed.\n>>\n>> Reported-by: Ilya Tumaykin <itumaykin@gmail.com>\n>> Helped-by: Jeff King <peff@peff.net>\n>> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n>> ---\n> \n> Well written.  Thanks for a pleasant read.\n\nThanks to Peff for letting me steal his commit message\n\n>> @@ -953,7 +960,7 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff,\n>>   \tcontext_line_count = 0;\n>>   \n>>   \twhile (splittable_into > 1) {\n>> -\t\tch = s->plain.buf[current];\n>> +\t\tch = normalize_marker(s->plain.buf + current);\n> \n> I wondered if &s->plain.buf[current] is easier to grok, but what's\n> written already is good enough ;-)\n\nYes I think that would be better\n\n> There is another explicit mention of ' ' in merge_hunks().  Unless\n> we are normalizing the buffer here (which I do not think we do),\n> wouldn't we have to make sure that the code over there also knows\n> that an empty line in a patch is an unmodified empty line?\n> \n>                  if (plain[overlap_end] != ' ')\n>                          return error(_(\"expected context line \"\n>                                         \"#%d in\\n%.*s\"),\n\nOh dear, I'm not sure how I missed that. I'll fix that and update the \ntest to make sure it exercises that code path as well.\n\nThanks for your comments\n\nPhillip\n\n\n"},{"id":"499004","messageId":"pull.1763.v2.git.1721491320.gitgitgadget@gmail.com","threadId":"61802","inReplyTo":"pull.1763.git.1721312619822.gitgitgadget@gmail.com","subject":"[PATCH v2 0/2] add-patch: handle splitting hunks with diff.suppressBlankEmpty","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-07-20T16:01:58Z","receivedAt":"2024-07-20T16:02:05Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"This is an alternative to jk/add-patch-with-suppress-blank-empty which was\nrecently discarded from next. I hope that normalizing the context marker\nwill simplify any future changes to the code.\n\nChanges since V1\n\n * Updated merge_hunks() to use normalize_marker() as spotted by Junio\n * Updated the test so it checks merge_hunks() as well.\n\nPhillip Wood (2):\n  add-patch: handle splitting hunks with diff.suppressBlankEmpty\n  add-patch: use normalize_marker() when recounting edited hunk\n\n add-patch.c                | 23 +++++++++++++++--------\n t/t3701-add-interactive.sh | 19 +++++++++++++++++++\n 2 files changed, 34 insertions(+), 8 deletions(-)\n\n\nbase-commit: 790a17fb19d6eadd16c52e5d284a5c6921744766\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1763%2Fphillipwood%2Fadd-p-suppress-blank-empty-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1763/phillipwood/add-p-suppress-blank-empty-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1763\n\nRange-diff vs v1:\n\n 1:  fd09e66727a ! 1:  34d8fd44a97 add-patch: handle splitting hunks with diff.suppressBlankEmpty\n     @@ add-patch.c: static void complete_file(char marker, struct hunk *hunk)\n       }\n       \n      +/* Empty context lines may omit the leading ' ' */\n     -+static int normalize_marker(char *p)\n     ++static int normalize_marker(const char *p)\n      +{\n      +\treturn p[0] == '\\n' || (p[0] == '\\r' && p[1] == '\\n') ? ' ' : p[0];\n      +}\n     @@ add-patch.c: static int parse_diff(struct add_p_state *s, const struct pathspec\n       \n       \t\tp = eol == pend ? pend : eol + 1;\n       \t\thunk->end = p - plain->buf;\n     +@@ add-patch.c: static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff,\n     + \t\t\t\t\t    (int)(hunk->end - hunk->start),\n     + \t\t\t\t\t    plain + hunk->start);\n     + \n     +-\t\t\t\tif (plain[overlap_end] != ' ')\n     ++\t\t\t\tif (normalize_marker(&plain[overlap_end]) != ' ')\n     + \t\t\t\t\treturn error(_(\"expected context line \"\n     + \t\t\t\t\t\t       \"#%d in\\n%.*s\"),\n     + \t\t\t\t\t\t     (int)(j + 1),\n      @@ add-patch.c: static int split_hunk(struct add_p_state *s, struct file_diff *file_diff,\n       \tcontext_line_count = 0;\n       \n       \twhile (splittable_into > 1) {\n      -\t\tch = s->plain.buf[current];\n     -+\t\tch = normalize_marker(s->plain.buf + current);\n     ++\t\tch = normalize_marker(&s->plain.buf[current]);\n       \n       \t\tif (!ch)\n       \t\t\tBUG(\"buffer overrun while splitting hunks\");\n     @@ t/t3701-add-interactive.sh: test_expect_success 'reset -p with unmerged files' '\n       \n      +test_expect_success 'hunk splitting works with diff.suppressBlankEmpty' '\n      +\ttest_config diff.suppressBlankEmpty true &&\n     -+\ttest_write_lines a b c \"\" d e f >file &&\n     ++\twrite_script fake-editor.sh <<-\\EOF &&\n     ++\ttr F G <\"$1\" >\"$1.tmp\" &&\n     ++\tmv \"$1.tmp\" \"$1\"\n     ++\tEOF\n     ++\n     ++\ttest_write_lines a b \"\" c d  \"\" e f \"\" >file &&\n      +\tgit add file &&\n     -+\ttest_write_lines p q r \"\" s t u >file &&\n     -+\ttest_write_lines s n y q | git add -p &&\n     ++\ttest_write_lines A b \"\" c D  \"\" e F \"\" >file &&\n     ++\t(\n     ++\t\ttest_set_editor \"$(pwd)/fake-editor.sh\" &&\n     ++\t\ttest_write_lines s n y e q | git add -p file\n     ++\t) &&\n      +\tgit cat-file blob :file >actual &&\n     -+\ttest_write_lines a b c \"\" s t u >expect &&\n     ++\ttest_write_lines a b \"\" c D \"\" e G \"\" >expect &&\n      +\ttest_cmp expect actual\n      +'\n      +\n -:  ----------- > 2:  7bdcd2df012 add-patch: use normalize_marker() when recounting edited hunk\n\n-- \ngitgitgadget\n"},{"id":"499005","messageId":"34d8fd44a97efd5a36003823f7db853291a2543c.1721491320.git.gitgitgadget@gmail.com","threadId":"61802","inReplyTo":"pull.1763.v2.git.1721491320.gitgitgadget@gmail.com","subject":"[PATCH v2 1/2] add-patch: handle splitting hunks with diff.suppressBlankEmpty","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-07-20T16:01:59Z","receivedAt":"2024-07-20T16:02:06Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nWhen \"add -p\" parses diffs, it looks for context lines starting with a\nsingle space. But when diff.suppressBlankEmpty is in effect, an empty\ncontext line will omit the space, giving us a true empty line. This\nconfuses the parser, which is unable to split based on such a line.\n\nIt's tempting to say that we should just make sure that we generate a\ndiff without that option.  However, although we do not parse hunks that\nthe user has manually edited with parse_diff() we do allow the user\nto split such hunks. As POSIX calls the decision of whether to print the\nspace here \"implementation-defined\" we need to handle edited hunks where\nempty context lines omit the space.\n\nSo let's handle both cases: a context line either starts with a space or\nconsists of a totally empty line by normalizing the first character to a\nspace when we parse them. Normalizing the first character rather than\nchanging the code to check for a space or newline will hopefully future\nproof against introducing similar bugs if the code is changed.\n\nReported-by: Ilya Tumaykin <itumaykin@gmail.com>\nHelped-by: Jeff King <peff@peff.net>\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n add-patch.c                | 19 +++++++++++++------\n t/t3701-add-interactive.sh | 19 +++++++++++++++++++\n 2 files changed, 32 insertions(+), 6 deletions(-)\n\ndiff --git a/add-patch.c b/add-patch.c\nindex d8ea05ff108..8feb719483f 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -400,6 +400,12 @@ static void complete_file(char marker, struct hunk *hunk)\n \t\thunk->splittable_into++;\n }\n \n+/* Empty context lines may omit the leading ' ' */\n+static int normalize_marker(const char *p)\n+{\n+\treturn p[0] == '\\n' || (p[0] == '\\r' && p[1] == '\\n') ? ' ' : p[0];\n+}\n+\n static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n {\n \tstruct strvec args = STRVEC_INIT;\n@@ -485,6 +491,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \twhile (p != pend) {\n \t\tchar *eol = memchr(p, '\\n', pend - p);\n \t\tconst char *deleted = NULL, *mode_change = NULL;\n+\t\tchar ch = normalize_marker(p);\n \n \t\tif (!eol)\n \t\t\teol = pend;\n@@ -532,7 +539,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\t\t * Start counting into how many hunks this one can be\n \t\t\t * split\n \t\t\t */\n-\t\t\tmarker = *p;\n+\t\t\tmarker = ch;\n \t\t} else if (hunk == &file_diff->head &&\n \t\t\t   starts_with(p, \"new file\")) {\n \t\t\tfile_diff->added = 1;\n@@ -586,10 +593,10 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)\n \t\t\t    (int)(eol - (plain->buf + file_diff->head.start)),\n \t\t\t    plain->buf + file_diff->head.start);\n \n-\t\tif ((marker == '-' || marker == '+') && *p == ' ')\n+\t\tif ((marker == '-' || marker == '+') && ch == ' ')\n \t\t\thunk->splittable_into++;\n-\t\tif (marker && *p != '\\\\')\n-\t\t\tmarker = *p;\n+\t\tif (marker && ch != '\\\\')\n+\t\t\tmarker = ch;\n \n \t\tp = eol == pend ? pend : eol + 1;\n \t\thunk->end = p - plain->buf;\n@@ -813,7 +820,7 @@ static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff,\n \t\t\t\t\t    (int)(hunk->end - hunk->start),\n \t\t\t\t\t    plain + hunk->start);\n \n-\t\t\t\tif (plain[overlap_end] != ' ')\n+\t\t\t\tif (normalize_marker(&plain[overlap_end]) != ' ')\n \t\t\t\t\treturn error(_(\"expected context line \"\n \t\t\t\t\t\t       \"#%d in\\n%.*s\"),\n \t\t\t\t\t\t     (int)(j + 1),\n@@ -953,7 +960,7 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff,\n \tcontext_line_count = 0;\n \n \twhile (splittable_into > 1) {\n-\t\tch = s->plain.buf[current];\n+\t\tch = normalize_marker(&s->plain.buf[current]);\n \n \t\tif (!ch)\n \t\t\tBUG(\"buffer overrun while splitting hunks\");\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 5d78868ac16..9a48933cecf 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -1164,4 +1164,23 @@ test_expect_success 'reset -p with unmerged files' '\n \ttest_must_be_empty staged\n '\n \n+test_expect_success 'hunk splitting works with diff.suppressBlankEmpty' '\n+\ttest_config diff.suppressBlankEmpty true &&\n+\twrite_script fake-editor.sh <<-\\EOF &&\n+\ttr F G <\"$1\" >\"$1.tmp\" &&\n+\tmv \"$1.tmp\" \"$1\"\n+\tEOF\n+\n+\ttest_write_lines a b \"\" c d  \"\" e f \"\" >file &&\n+\tgit add file &&\n+\ttest_write_lines A b \"\" c D  \"\" e F \"\" >file &&\n+\t(\n+\t\ttest_set_editor \"$(pwd)/fake-editor.sh\" &&\n+\t\ttest_write_lines s n y e q | git add -p file\n+\t) &&\n+\tgit cat-file blob :file >actual &&\n+\ttest_write_lines a b \"\" c D \"\" e G \"\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"499006","messageId":"7bdcd2df01246932e452417815368f6c56d83e8d.1721491320.git.gitgitgadget@gmail.com","threadId":"61802","inReplyTo":"pull.1763.v2.git.1721491320.gitgitgadget@gmail.com","subject":"[PATCH v2 2/2] add-patch: use normalize_marker() when recounting edited hunk","fromName":"Phillip Wood via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-07-20T16:02:00Z","receivedAt":"2024-07-20T16:02:08Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nAfter the user has edited a hunk the number of lines in the pre- and\npost- image lines is recounted the hunk header can be updated before\npassing the hunk to \"git apply\". The recounting code correctly handles\nempty context lines where the leading ' ' is omitted by treating '\\n'\nand '\\r' as context lines.\n\nUpdate this code to use normalize_marker() so that the handling of empty\ncontext lines is consistent with the rest of the hunk parsing\ncode. There is a small change in behavior as normalize_marker() only\ntreats \"\\r\\n\" as an empty context line rather than any line starting\nwith '\\r'. This should not matter in practice as Macs have used Unix\nline endings since MacOs 10 was released in 2001 and if it transpires\nthat someone is still using an earlier version of MacOs where lines end\nwith '\\r' then we will need to change the handling of '\\r' in\nnormalize_marker() anyway.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n add-patch.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/add-patch.c b/add-patch.c\nindex 8feb719483f..2a72ad63d14 100644\n--- a/add-patch.c\n+++ b/add-patch.c\n@@ -1178,14 +1178,14 @@ static ssize_t recount_edited_hunk(struct add_p_state *s, struct hunk *hunk,\n \n \theader->old_count = header->new_count = 0;\n \tfor (i = hunk->start; i < hunk->end; ) {\n-\t\tswitch (s->plain.buf[i]) {\n+\t\tswitch(normalize_marker(&s->plain.buf[i])) {\n \t\tcase '-':\n \t\t\theader->old_count++;\n \t\t\tbreak;\n \t\tcase '+':\n \t\t\theader->new_count++;\n \t\t\tbreak;\n-\t\tcase ' ': case '\\r': case '\\n':\n+\t\tcase ' ':\n \t\t\theader->old_count++;\n \t\t\theader->new_count++;\n \t\t\tbreak;\n-- \ngitgitgadget\n"}]}