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

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: got patch: add flag to ignore whitespace?
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Sat, 2 Jul 2022 20:08:30 +0200

Download raw body.

Thread
On Sat, Jul 02, 2022 at 05:27:30PM +0200, Omar Polo wrote:
> Omar Polo <op@omarpolo.com> wrote:
> > The last patch from Mark reminded me that I thought some time ago to add
> > a "-w" flag for got patch to ignore white spaces.  It's conceptually
> > similar to patch(1) -l/--ignore-whitespace or git-apply(1)
> > --ignore-whitespace.
> > 
> > Here's a possible usage (using Mark' last diff)
> > 
> > 	% mshow | got patch
> > 	#  tog/tog.1
> > 	@@ -295,7 +305,7 @@ hunk failed to apply
> > 	G  tog/tog.c
> > 	got: patch failed to apply
> > 	% # woops, try again with -w
> > 	% mshow | got patch -w
> > 	G  tog/tog.1
> > 	G  tog/tog.c
> > 	% # yay!
> > 
> > The diff does exactly what one would expect.  The line comparisons are
> > now done in linecmp, which first runs strcmp(3) and optionally falls
> > back to a whitespace-loose matching.
> > 
> > There's a bit of churn in apply_hunk now.  Before, I was re-using the
> > lines saved in the hunk to apply it, now I can't.  If the context lines
> > of the hunk may have the whitespaces mangled and so I have to re-read
> > the lines from the original file.  Even if we're using a loose match,
> > the context line needs not to be changed.
> 
> Wops, it introduced a stupid error in apply_hunk: when adding a file we
> wouldn't add \n at the end of the lines.  The regress suite didn't catch
> this, I'll add a testcase for it later as it's independent from this
> diff
> 
> The change with the previous is to strip newlines read in apply_hunk and
> always add them when printing to the temp file.

Nice idea.

Recalling that patch(1) is supposed to cope with fuzzyness of changes,
I wonder if -w as you implemented it here should be the default behaviour?
With an option (maybe -w) to turn strict whitespace matching back on?

In cases where one would need to use the -w option as you have implemented it
there is no other way to get the diff to apply. So we could make your -w the
default and provide a visual hint when whitespace differences are being
ignored:

 	@@ -295,7 +305,7 @@ hunk contains mangled whitespace

And if anyone is for some reason worried about applying such mangled
whitespace diffs they could use an option to ask for strict
whitespace matching instead.