Re: [PATCH] whitespace: symbolic links usually lack LF at the end
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Feb 5, 2026, 12:21 UTC
- Message-ID
- <aYSLP1LqBiMwur3O@pks.im>
- In-Reply-To
- <xmqqecn0nqyt.fsf@gitster.g>
On Wed, Feb 04, 2026 at 01:23:06PM -0800, Junio C Hamano wrote:
Show 16 quoted lines
> diff --git a/apply.c b/apply.c > index 3de4aa4d2e..581aafb8be 100644 > --- a/apply.c > +++ b/apply.c > @@ -2193,6 +2193,11 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si > patch->ws_rule = whitespace_rule(state->repo->index, > patch->old_name); > > + /* being an incomplete line is the norm for a symbolic link */ > + if ((patch->old_mode && S_ISLNK(patch->old_mode)) || > + (patch->new_mode && S_ISLNK(patch->new_mode))) > + patch->ws_rule &= ~WS_INCOMPLETE_LINE; > + > patchsize = parse_single_patch(state, > buffer + offset + hdrsize, > size - offset - hdrsize,
Hm. Wouldn't that mean that we disable this check for both sides of a diff if either of them is a symlink? That's typically fine, but if the diff also contains a mode change it might not be.
I'd suggest that we only disable this check in case either:
- One side doesn't exist, the other is a symbolic link.
- Both sides are a symbolic link.
Another question is whether we support symref targets that end in a newline. I guess the answer is going to be some form of "yes", and in that case we could of course loose some information. But honestly, this is so much of an edge case that I don't really worry about it too much.
Thanks!
Patrick