"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: got patch: handle "\ No newline at end of file"
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Sat, 19 Mar 2022 17:57:45 +0100

Download raw body.

Thread
On Sat, Mar 19, 2022 at 05:48:05PM +0100, Omar Polo wrote:
> This is the last "big" missing piece for 'got patch': handle the "\ No
> newline at end of file".
> 
> The implementation is quite simple: avoid printing the last newline
> character if the patch says so.
> 
> I'm now chopping off all the final newline character when reading lines
> (either from the patchfile or from the "original" file): this keeps the
> locate_hunk and test_hunk functions (the hunk matching routines) working
> in every case.
> 
> The only consequence is that we don't fail to match a hunk if the
> original file has a trailing newline and patchfile says that it
> shouldn't.  I don't think it's a great deal, the "\ Now newline at eof"
> indicator is only allowed at the end so we don't risk to apply the patch
> at a different offset, and it's common for editors to add a trailing
> newline anyway (Emacs has a `require-final-newline' setting and mg(1)
> always prompts to add a newline if missing when saving.)
> 
> The only vaguely tricky part is the parsing.  The hunk header (the line
> that starts with "@@") has some counters for the number of following
> lines that belong to the old or new file, but the "\ No final newline"
> line is not accounted for in there.  It's only allowed after the last
> '+' or '-' thought, so it's not that bad.  I'm peeking one character in
> peek_special_line and either consume the line or put that back.
> ungetc(3) says that one character of push-back is guaranteed, so it
> should be safe.  It's fundamentally the same thing Larry Wall' patch
> does, grep for `remove_special_line' in usr.bin/patch/pch.c
> 
> The patch also include a small tweak that I'll probably land as a
> separate commit: reuse apply_hunk instead of rolling the loop again in
> patch_file for the "create file" case.
> 
> ok?

Should we handle CRLF newlines? It seems your code would split such
lines at LF, leaving a trailing CR in place.
Or do we only assume unix semantics and support just \n, not \r\n?

Does Larry's patch even handle CRLF?

There is also an older(?) convention on Mac that uses just \r for newlines.

FWIW, svn patch supports all of these, but only because every part
of SVN has to support them. We don't necessarily have to do the same,
but we should make a decision and document it.