On Mon, Aug 26, 2024 at 05:35:14PM -0700, Junio C Hamano wrote:
> Rubén Justo <rjusto@xxxxxxxxx> writes:
>
> > Ensure strict matching of context lines when applying with
> > `--whitespace=fix` combined with `--no-ignore-whitespace`.
> >
> > Signed-off-by: Rubén Justo <rjusto@xxxxxxxxx>
> > ---
> > apply.c | 3 ++-
> > t/t4124-apply-ws-rule.sh | 27 +++++++++++++++++++++++++++
> > 2 files changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/apply.c b/apply.c
> > index 63e58086f1..0cb9d38e5a 100644
> > --- a/apply.c
> > +++ b/apply.c
> > @@ -2596,7 +2596,8 @@ static int match_fragment(struct apply_state *state,
> > goto out;
> > }
> >
> > - if (state->ws_error_action != correct_ws_error) {
> > + if (state->ws_error_action != correct_ws_error ||
> > + state->ws_ignore_action == ignore_ws_none) {
> > ret = 0;
> > goto out;
> > }
>
> Hmph, if we are correcting for whitespace violations, even if
> whitespace fuzz is not allowed externally, wouldn't the issue that
> c1beba5b (git-apply --whitespace=fix: fix whitespace fuzz introduced
> by previous run, 2008-01-30) corrected still apply? IOW, isn't this
> change introducing a regression when an input touches a file with a
> change with broken whitespaces, and then touches the same file to
> replace the broken whitespace lines with something else?
Yes, that is the center of the blast this series is producing;
affecting to what `--whitespace=fix` does (just a reminder for other
readers):
- stop fixing whitespace errors in context lines and
- no longer warning about them.
Clearly, as you point out, the change poses a problem when applying
changes with whitespace errors in lines involved multiple times,
either during the same apply session or across multiple sessions
executed in sequence.
The new `ignore_ws_default` enum option is intended to mitigate the
blast. It would be unexpected IMHO for someone who wants the behavior
described in c1beba5b to be indicating `--no-ignore-whitespace`. And
with it we allow the possibility for someone who finds that behavior
undesirable to avoid it.
I'm not very happy with the new enum, but I haven't come up with a
better idea. There are other alternatives:
--whitespace=fix-strict
--do-not-fix-ws-errors-in-context-lines
None of them are better, I think.
