From: Phillip Wood Date: Tue, 11 Nov 2025 14:29:15 GMT Subject: Re: [PATCH v3 00/12] Incomplete lines Message-ID: In-Reply-To: <20251111000451.2243195-1-gitster@pobox.com> On 11/11/2025 00:04, Junio C Hamano wrote: > Changes in v3: > > - The proposed log message of [PATCH 05/12] explains that it > semi-duplicates the same code shared in two case arems in > preparation for later changes. > > - The internal helper function to emit the "\ No newline" marker > line is now called emit_incomplete_line_marker(). > > - Two conditionals in [PATCH 07/12] both of which switched on !endp > have been consolidated into a single if/else statement. > > - The tests in [PATCH 11/12] checks the output from "diff --check" > now. The range-diff looks good. I'm still not keen on patch 4 but all the other diff related changes look fine to me. Thanks Phillip > > Changes in v2: > > - rolled the definition (but not implementation) of the new > "incomplete-line" class into step [09/12] that shifts the bit > assignment. The documentation of core.whitespace has also be > updated in this step. > > - "git apply --check" miscounted line number reported for the > incomplete line error, which has been corrected in step [10/12]. > > - t4124-apply-ws-rule.sh has been extended to cover "git apply > --check" and the diagnostic output from it in step [10/12]. > > Junio C Hamano (12): > whitespace: correct bit assignment comments > diff: emit_line_ws_markup() if/else style fix > diff: correct suppress_blank_empty hack > diff: fix incorrect counting of line numbers > diff: refactor output of incomplete line > diff: call emit_callback ecbdata everywhere > diff: update the way rewrite diff handles incomplete lines > apply: revamp the parsing of incomplete lines > whitespace: allocate a few more bits and define WS_INCOMPLETE_LINE > apply: check and fix incomplete lines > diff: highlight and error out on incomplete lines > attr: enable incomplete-line whitespace error for this project > > .gitattributes | 4 +- > Documentation/config/core.adoc | 2 + > apply.c | 79 ++++++++++---- > diff.c | 152 ++++++++++++++++++--------- > diff.h | 6 +- > t/t4015-diff-whitespace.sh | 67 +++++++++++- > t/t4124-apply-ws-rule.sh | 187 +++++++++++++++++++++++++++++++++ > ws.c | 20 ++++ > ws.h | 26 +++-- > 9 files changed, 455 insertions(+), 88 deletions(-) > > Range-diff against v2: > 1: c045e93ce5 = 1: c045e93ce5 whitespace: correct bit assignment comments > 2: 0d95d68fb4 = 2: 0d95d68fb4 diff: emit_line_ws_markup() if/else style fix > 3: c331218334 = 3: c331218334 diff: correct suppress_blank_empty hack > 4: be1473fc5a = 4: be1473fc5a diff: fix incorrect counting of line numbers > 5: 7bcd6efba8 ! 5: 9410e4257a diff: refactor output of incomplete line > @@ Commit message > the code path that handles xdiff output and the code path that > bypasses xdiff and produces complete rewrite patch. > > + Currently the output from the DIFF_SYMBOL_CONTEXT_INCOMPLETE case > + still (ab)uses the same code as what is used for context lines, but > + that would change in a later step where we introduce support for > + incomplete line detection. > + > Signed-off-by: Junio C Hamano > > ## diff.c ## > @@ diff.c: static void emit_context_line(struct emit_callback *ecbdata, > emit_diff_symbol(ecbdata->opt, DIFF_SYMBOL_CONTEXT, line, len, flags); > } > > -+static void emit_incomplete_line(struct emit_callback *ecbdata, > -+ const char *line, int len) > ++static void emit_incomplete_line_marker(struct emit_callback *ecbdata, > ++ const char *line, int len) > +{ > + emit_diff_symbol(ecbdata->opt, DIFF_SYMBOL_CONTEXT_INCOMPLETE, > + line, len, 0); > @@ diff.c: static int fn_out_consume(void *priv, char *line, unsigned long len) > } > - emit_diff_symbol(o, DIFF_SYMBOL_CONTEXT_INCOMPLETE, > - line, len, 0); > -+ emit_incomplete_line(ecbdata, line, len); > ++ emit_incomplete_line_marker(ecbdata, line, len); > break; > default: > BUG("fn_out_consume: unknown line '%s'", line); > 6: 1a6f143377 = 6: cdc6516009 diff: call emit_callback ecbdata everywhere > 7: dfc810b1d6 ! 7: 9acb9b6217 diff: update the way rewrite diff handles incomplete lines > @@ diff.c: static void emit_rewrite_lines(struct emit_callback *ecbdata, > + char *pdata = NULL; > > endp = memchr(data, '\n', size); > - len = endp ? (endp - data + 1) : size; > -+ plen = len; > +- len = endp ? (endp - data + 1) : size; > + > -+ if (!endp) { > ++ if (endp) { > ++ len = endp - data + 1; > ++ plen = len; > ++ } else { > ++ len = size; > + plen = len + 1; > + pdata = xmalloc(plen + 2); > + memcpy(pdata, data, len); > @@ diff.c: static void emit_rewrite_lines(struct emit_callback *ecbdata, > + if (!endp) { > + static const char nneof[] = "\\ No newline at end of file\n"; > + ecbdata->last_line_kind = prefix; > -+ emit_incomplete_line(ecbdata, nneof, sizeof(nneof) - 1); > ++ emit_incomplete_line_marker(ecbdata, nneof, sizeof(nneof) - 1); > + } > } > > 8: c66b547f13 = 8: 86c14ee62d apply: revamp the parsing of incomplete lines > 9: bdc2dbbe4b = 9: b62d4020e7 whitespace: allocate a few more bits and define WS_INCOMPLETE_LINE > 10: 806aa30511 = 10: 081c21b14e apply: check and fix incomplete lines > 11: 0cfb6ab295 ! 11: 73182b19a8 diff: highlight and error out on incomplete lines > @@ diff.c: static void emit_diff_symbol_from_struct(struct diff_options *o, > emit_line(o, set, reset, line, len); > break; > @@ diff.c: static void emit_context_line(struct emit_callback *ecbdata, > - static void emit_incomplete_line(struct emit_callback *ecbdata, > - const char *line, int len) > + static void emit_incomplete_line_marker(struct emit_callback *ecbdata, > + const char *line, int len) > { > + int last_line_kind = ecbdata->last_line_kind; > + unsigned flags = (last_line_kind == '+' > @@ t/t4015-diff-whitespace.sh: do > +' > + > +test_expect_success "incomplete lines on both pre- and post-image" ' > -+ # The interpretation taken here is "since you are toucing > ++ # The interpretation taken here is "since you are touching > + # the line anyway, you would better fix the incomplete line > + # while you are at it." but this is debatable. > + echo foo | tr -d "\012" >x && > + git add x && > + echo bar | tr -d "\012" >x && > + git diff x && > -+ test_must_fail git -c core.whitespace=incomplete diff --check x && > ++ test_must_fail git -c core.whitespace=incomplete diff --check x >error && > ++ test_grep "no newline at the end of file" error && > + git diff -R x && > -+ test_must_fail git -c core.whitespace=incomplete diff -R --check x > ++ test_must_fail git -c core.whitespace=incomplete diff -R --check x >error && > ++ test_grep "no newline at the end of file" error > +' > + > +test_expect_success "fix incomplete line in pre-image" ' > @@ t/t4015-diff-whitespace.sh: do > + git diff x && > + git -c core.whitespace=incomplete diff --check x && > + git diff -R x && > -+ test_must_fail git -c core.whitespace=incomplete diff -R --check x > ++ test_must_fail git -c core.whitespace=incomplete diff -R --check x >error && > ++ test_grep "no newline at the end of file" error > +' > + > +test_expect_success "new incomplete line in post-image" ' > @@ t/t4015-diff-whitespace.sh: do > + git add x && > + echo bar | tr -d "\012" >x && > + git diff x && > -+ test_must_fail git -c core.whitespace=incomplete diff --check x && > ++ test_must_fail git -c core.whitespace=incomplete diff --check x >error && > ++ test_grep "no newline at the end of file" error && > + git diff -R x && > + git -c core.whitespace=incomplete diff -R --check x > +' > 12: 33c5ae40db = 12: 85748701b4 attr: enable incomplete-line whitespace error for this project