The added test would hang up Git due to an infinite loop. The function
`next_byte()` doesn't make any forward progress in the buffer with
`--ignore-space-change`.
Fix this by only returning early when there was actual white space
to be covered, fall back to the default case at the end of the funct
On Thu, Oct 12, 2017 at 04:33:22PM -0700, Stefan Beller wrote:
> Peff, feel free to take ownership here. I merely made it to a patch.
I think compared to my original, it makes sense to actually wrap the
whole thing with a check for the whitespace. You can do it just in the
IGNORE_WHITESPACE_CHANG
On Thu, Oct 12, 2017 at 08:18:37PM -0400, Jeff King wrote:
> Fix this by entering the conditional only when we actually
> see whitespace. We can apply this also to the
> IGNORE_WHITESPACE change. That code path isn't buggy
> (because it falls through to returning the next
> non-whitespace byte), b
On Thu, Oct 12, 2017 at 5:20 PM, Jeff King wrote:
> On Thu, Oct 12, 2017 at 08:18:37PM -0400, Jeff King wrote:
>
>> Fix this by entering the conditional only when we actually
>> see whitespace. We can apply this also to the
>> IGNORE_WHITESPACE change. That code path isn't buggy
>> (because it fal
On Thu, Oct 12, 2017 at 08:20:57PM -0400, Jeff King wrote:
> On Thu, Oct 12, 2017 at 08:18:37PM -0400, Jeff King wrote:
>
> > Fix this by entering the conditional only when we actually
> > see whitespace. We can apply this also to the
> > IGNORE_WHITESPACE change. That code path isn't buggy
> > (
On Thu, Oct 19, 2017 at 01:04:59AM -0400, Jeff King wrote:
> So. That leaves me with:
>
> - I'm unclear on whether next_byte() is meant to return that trailing
> NUL or not. I don't think it causes any bugs, but it certainly
> confused me for a function to take a cp/endp pair of pointer
Jeff King writes:
> it does. It just adjusts our "end pointer" to point to the last valid
> character in the string (rather than one past), which seems to be the
> convention that those loops (and next_byte) expect.
Yeah I am not sure if I like this comparison at the beginning of the
function:
Junio C Hamano writes:
> Jeff King writes:
>
>> it does. It just adjusts our "end pointer" to point to the last valid
>> character in the string (rather than one past), which seems to be the
>> convention that those loops (and next_byte) expect.
>
> Yeah I am not sure if I like this comparison a
On Thu, Oct 19, 2017 at 02:32:04PM +0900, Junio C Hamano wrote:
> > Yeah I am not sure if I like this comparison at the beginning of the
> > function:
> >
> > static int next_byte(const char **cp, const char **endp,
> > const struct diff_options *diffopt)
> >
On Thu, Oct 19, 2017 at 02:30:08PM +0900, Junio C Hamano wrote:
> Jeff King writes:
>
> > it does. It just adjusts our "end pointer" to point to the last valid
> > character in the string (rather than one past), which seems to be the
> > convention that those loops (and next_byte) expect.
>
> Y
On Wed, Oct 18, 2017 at 10:24 PM, Jeff King wrote:
> On Thu, Oct 19, 2017 at 01:04:59AM -0400, Jeff King wrote:
>
>> So. That leaves me with:
>>
>> - I'm unclear on whether next_byte() is meant to return that trailing
>> NUL or not. I don't think it causes any bugs, but it certainly
>> c
On Thu, Oct 19, 2017 at 12:53:03PM -0700, Stefan Beller wrote:
> > @@ -771,7 +771,7 @@ static unsigned get_string_hash(struct
> > emitted_diff_symbol *es, struct diff_opti
> > {
> > if (o->xdl_opts & XDF_WHITESPACE_FLAGS) {
> > static struct strbuf sb = STRBUF_INIT;
> > -
On Wed, Oct 18, 2017 at 10:42 PM, Jeff King wrote:
>
> So I think the right fix is this:
>
[...]
>
> It's late here, so I'll wait for comments from Stefan and then try to
> wrap it up with a commit message and test tomorrow.
>
> -Peff
I agree that this is better and looks correct.
Thanks for off
13 matches
Mail list logo
