Re: [PATCH v3 00/12] Incomplete lines

[Phillip Wood <phillip.wood123@gmail.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 <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