On Sun, Jul 03, 2022 at 09:24:10AM +0200, Omar Polo wrote:
> However, this got me thinking: maybe ignoring _every_ whitespace is too
> lax?  What about ignoring only the leading and trailing ones?  Regress
> passes and the mangled patch sent by Mark still applies.

Hmm. Not sure about leading whitespace, this could get complicated quickly.

Some email clients convert tabs ('\t') to spaces (' ') and mangle patches.
We should have a regress test involving code which is indented by tabs
in the original and target file, and indented by spaces in the patch.
It would be difficult for 'got patch' to repair such indentation problems
automatically and meet user expectations. Some styles, including our own,
mix tabs and spaces on the same line. 'got patch' would have to infer the
indentation style from the target file that is being patched, and then apply
lines with bad whitespace such that tabs vs. spaces are preserved in a
way the user expects. Which is impossible to get right in general, since
we don't know which whitespace was added by the author or the email program.
This may be why other patch tools leave this behaviour off by default.

Because this is just whitespace, and our output already hints that whitespace
was mangled, I still think -w should be the default for 'got patch'. But
we should definitely provide an option to disable it.

Another problem with having -w on by default is that it is possible to
encode harmful things in unicode which end up rendering only whitespace
on the terminal. We should make sure that we are really looking at ASCII
characters ' ' and \t, instead of using isspace() (which may do
who-knows-what on systems running got-portable).