Re: [PATCH v3 00/12] Incomplete lines
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Nov 11, 2025, 14:29 UTC
- Message-ID
- <bc2b7b52-a8f9-473c-a6fe-2c0d3fa089ea@gmail.com>
- In-Reply-To
- <20251111000451.2243195-1-gitster@pobox.com>
On 11/11/2025 00:04, Junio C Hamano wrote:
Show 14 quoted lines
> 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
Show 163 quoted lines
>
> 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 <gitster@pobox.com>
>
> ## 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