Re: [PATCH v2 07/12] diff: update the way rewrite diff handles incomplete lines
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Nov 10, 2025, 10:06 UTC
- Message-ID
- <aRG5NSvzeie_J3x9@pks.im>
- In-Reply-To
- <20251105213052.1499224-8-gitster@pobox.com>
On Wed, Nov 05, 2025 at 01:30:47PM -0800, Junio C Hamano wrote:
Show 5 quoted lines
> The diff_symbol based output framework uses one DIFF_SYMBOL_* enum > value per the kind of output lines of "git diff", which corresponds > to one output line from the xdiff machinery used internally. Most > notably, DIFF_SYMBOL_PLUS and DIFF_SYMBOL_MINUS that correspond to > "+" and "-" lines are designed to always take a complete line, even
"complete line" as in newline-terminated? I only recognized that this is what you meant in the next paragraph, so it might be useful to clarify here already what you mean.
Show 44 quoted lines
> diff --git a/diff.c b/diff.c
> index 347cd9c6e9..99298720f4 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1786,22 +1777,36 @@ static void emit_rewrite_lines(struct emit_callback *ecbdata,
> const char *endp = NULL;
>
> while (0 < size) {
> - int len;
> + int len, plen;
> + char *pdata = NULL;
>
> endp = memchr(data, '\n', size);
> len = endp ? (endp - data + 1) : size;
> + plen = len;
> +
> + if (!endp) {
> + plen = len + 1;
> + pdata = xmalloc(plen + 2);
> + memcpy(pdata, data, len);
> + pdata[len] = '\n';
> + pdata[len + 1] = '\0';
> + }
> if (prefix != '+') {
> ecbdata->lno_in_preimage++;
> - emit_del_line(ecbdata, data, len);
> + emit_del_line(ecbdata, pdata ? pdata : data, plen);
> } else {
> ecbdata->lno_in_postimage++;
> - emit_add_line(ecbdata, data, len);
> + emit_add_line(ecbdata, pdata ? pdata : data, plen);
> }
> + free(pdata);
> size -= len;
> data += len;
> }
> - if (!endp)
> - emit_diff_symbol(ecbdata->opt, DIFF_SYMBOL_NO_LF_EOF, NULL, 0, 0);
> + 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);
> + }
> }Okay. I was wondering at first how this would get executed for both pre- and postimage if it's not part of the loop anymore. But this is mostly showing my complete ignorance for the "diff" subsystem, as we end up calling `emit_rewrite_lines()` itself once for each image.
Patrick