{"thread":{"id":"23381","subject":"[PATCH] apply: Allow blank *trailing* context lines to match beyond EOF","startedAt":"2010-04-08T04:14:31Z","lastAt":"2010-04-08T05:32:04Z","messageCount":2,"participants":["Björn Gustavsson","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"138917","messageId":"4BBD5827.7030003@gmail.com","threadId":"23381","inReplyTo":null,"subject":"[PATCH] apply: Allow blank *trailing* context lines to match beyond EOF","fromName":"Björn Gustavsson","fromEmail":"bgustavsson@gmail.com","sentAt":"2010-04-08T04:14:31Z","receivedAt":"2010-04-08T04:14:31Z","isPatch":true,"sender":{"key":"bgustavsson@gmail.com","avatar":"https://avatars.githubusercontent.com/u/74840?v=4"},"body":"In 51667147be, \"git apply --whitespace=fix\" was extended to\nallow a blank context line to match beyond the end of the file,\nbut only if the context line was in the leading part of the\nhunk (i.e. the hunk inserted additional contents at the end\nof the file).\n\nDrop the restriction that the context line must be in the\nleading part of the hunk, thus allowing a file to be changed\nfrom:\n\n a\n (blank line)\n\nto:\n\n b\n a\n (blank line)\n\nNote that the blank line will be kept, because \"--whitespace=fix\"\nonly removes trailing blank lines that a hunk would add, never\ntrailing blank lines in the context.\n\nSigned-off-by: Björn Gustavsson <bgustavsson@gmail.com>\n---\nThis patch should fix the problem observed by Junio, but note that\nthere will be one or more blank lines left at the end of the\nfile.\n\nI am not sure whether that should be fixed. In this particular\ncase, the blank lines are part of the context but not part of\nthe file being patched, so it could be argued that the blank\nlines should not be added back.\n\nBut there are already other circumstances in which\n\"--whitespace=fix\" does not guarantee that the file does\nnot end with blank lines, for instance if we have:\n\n a\n b\n (blank line)\n (blank line)\n (blank line)\n (blank line)\n c\n d\n\nand then delete the \"c\" and \"d\" lines:\n\n a\n b\n (blank line)\n (blank line)\n (blank line)\n (blank line)\n\nIn this case, not all of the blank lines are even part of the context,\nso even if we'll change the rules and remove blanks line in the context,\nthere would still be blank lines left at the end of the file.\n\n\n builtin/apply.c          |   12 ++++++------\n t/t4124-apply-ws-rule.sh |   12 ++++++++++++\n 2 files changed, 18 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 7ca9047..d17e046 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -1864,13 +1864,13 @@ static int match_fragment(struct image *img,\n \t\tif (match_end && (preimage->nr + try_lno != img->nr))\n \t\t\treturn 0;\n \t} else if (ws_error_action == correct_ws_error &&\n-\t\t   (ws_rule & WS_BLANK_AT_EOF) && match_end) {\n+\t\t   (ws_rule & WS_BLANK_AT_EOF)) {\n \t\t/*\n-\t\t * This hunk that matches at the end extends beyond\n-\t\t * the end of img, and we are removing blank lines\n-\t\t * at the end of the file.  This many lines from the\n-\t\t * beginning of the preimage must match with img, and\n-\t\t * the remainder of the preimage must be blank.\n+\t\t * This hunk extends beyond the end of img, and we are\n+\t\t * removing blank lines at the end of the file.  This\n+\t\t * many lines from the beginning of the preimage must\n+\t\t * match with img, and the remainder of the preimage\n+\t\t * must be blank.\n \t\t */\n \t\tpreimage_limit = img->nr - try_lno;\n \t} else {\ndiff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh\nindex fb9ad24..451d75e 100755\n--- a/t/t4124-apply-ws-rule.sh\n+++ b/t/t4124-apply-ws-rule.sh\n@@ -325,6 +325,18 @@ test_expect_success 'two missing blank lines at end with --whitespace=fix' '\n \ttest_cmp one expect\n '\n \n+test_expect_success 'missing blank line at end, insert before end, --whitespace=fix' '\n+\t{ echo a; echo; } >one &&\n+\tgit add one &&\n+\t{ echo b; echo a; echo; } >one &&\n+\tcp one expect &&\n+\tgit diff -- one >patch &&\n+\techo a >one &&\n+\ttest_must_fail git apply patch &&\n+\tgit apply --whitespace=fix patch &&\n+\ttest_cmp one expect\n+'\n+\n test_expect_success 'shrink file with tons of missing blanks at end of file' '\n \t{ echo a; echo b; echo c; } >one &&\n \tcp one no-blank-lines &&\n-- \n1.7.0.2.157.gb7e7f\n"},{"id":"138924","messageId":"7vd3ya8q7f.fsf@alter.siamese.dyndns.org","threadId":"23381","inReplyTo":"4BBD5827.7030003@gmail.com","subject":"Re: [PATCH] apply: Allow blank *trailing* context lines to match beyond EOF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-04-08T05:32:04Z","receivedAt":"2010-04-08T05:32:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Björn Gustavsson <bgustavsson@gmail.com> writes:\n\n> This patch should fix the problem observed by Junio, but note that\n> there will be one or more blank lines left at the end of the\n> file.\n>\n> I am not sure whether that should be fixed. In this particular\n> case, the blank lines are part of the context but not part of\n> the file being patched, so it could be argued that the blank\n> lines should not be added back.\n>\n> But there are already other circumstances in which\n> \"--whitespace=fix\" does not guarantee that the file does\n> not end with blank lines, for instance if we have:\n>\n>  a\n>  b\n>  (blank line)\n>  (blank line)\n>  (blank line)\n>  (blank line)\n>  c\n>  d\n>\n> and then delete the \"c\" and \"d\" lines:\n>\n>  a\n>  b\n>  (blank line)\n>  (blank line)\n>  (blank line)\n>  (blank line)\n\nEven though I think these are both worth fixing, I do not think it should\nhappen as part of this patch.  Because the code that needs to \"fix\" the\n\"extra blank bug\" this patch introduces will need to deal with exactly the\nsame horizon effect as your \"deleting c and d at the end will not have the\nblank immediately after b in the context\" example, I expect we will fix\nthe fallout from this patch when we fix that \"delete c and d at the end\"\nexample.\n"}]}