Re: [PATCH 3/5] apply: whitespace errors in context lines if we have

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Hmph.  0a80bc9f (apply: detect and mark whitespace errors in context
> lines when fixing, 2015-01-16) deliberately added this check because
> we will correct the whitespace breakages on these lines after
> parsing the hunk with this function while applying.
>
> It is iffy that this case arm for " " kicks in ONLY when applying in
> the forward direction (which is not what you are changing).  When
> applying a patch in reverse, " " is still an "unchanged" context
> line, so we should be treating it the same way regardless of the
> direction.
>
> But at least the call to check_whitespace() from this place when we
> are correcting whitespace rule violations is not iffy, as far as I
> can tell.

Having said all that, I do have to wonder how much value we are
getting by supporting that odd "feature" that makes apply take input
in a single session a patch that touches the same path TWICE.

If we can get rid of that feature (which I consider a misfeature),
we can lose quote a lot of code (anything that touches fn_table can
go) and recover the code quality that got visibly worse with the
addition of that feature back.

And without the "input may touch the same path TWICE", we do not
have to worry about this "context lines after applying a single
patch with whitespace=fix will have to be matched loosely with
respect to the whitespace when another patch modifies the same file
around the same lines", making your changes in [3/5] trivially the
right thing to do.

So, I am inclined to say that

 * we propose to get rid of that "a single input may touch the same
   path TWICE" feature at Git 3.0 boundary.

 * we at the same time apply [3/5] (and possibly others, but I do
   not think we want [1/5]).

But until we can shed our pretense that the "single input may touch
the same path TWICE" is seriously supported, I do not think applying
this series as-is makes sense, as it directly contradicts with that
(mis)feature.






[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux