[thomasbachem]

Cc: Johannes Schindelin Johannes.Schindelin@gmx.de
Cc: Junio C Hamano gitster@pobox.com
cc: Tuomas Ahola taahol@utu.fi

[thomasbachem]

/preview

[gitgitgadget]

Preview email sent as pull.2261.git.1792148387473.gitgitgadget@gmail.com

[thomasbachem]

/submit

[gitgitgadget]

Submitted as pull.2261.git.1792154374653.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2261/thomasbachem/sideband-control-characters-loop-v1

To fetch this version to local tag pr-2261/thomasbachem/sideband-control-characters-loop-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2261/thomasbachem/sideband-control-characters-loop-v1

[gitgitgadget]

Tuomas Ahola wrote on the Git mailing list (how to reply to this email):

"Thomas Bachem via GitGitGadget" <gitgitgadget@gmail.com> wrote:

> +			warning(_("unrecognized value for '%s': '%.*s'"),
> +				var, (int)(end - value), value);
> +			value = end + !!*end;
> +		}

Reading CodingGuidelines, it seems that `!!` is a construct we would
like to avoid:

 - Some clever tricks, like using the !! operator with arithmetic
   constructs, can be extremely confusing to others.  Avoid them,
   unless there is a compelling reason to use them.

--Tuomas

[gitgitgadget]

User Tuomas Ahola <taahol@utu.fi> has been added to the cc: list.

[gitgitgadget]

Thomas Bachem wrote on the Git mailing list (how to reply to this email):

Tuomas Ahola <taahol@utu.fi> wrote:
>> + value = end + !!*end;
>
> Reading CodingGuidelines, it seems that `!!` is a construct we would
> like to avoid:

I wrote it this way to match skip_prefix_in_csv() a few lines above,
which steps past the comma the same way:

*out = value + !!*value;

so the two places in this parser read alike. Happy to spell it out as a
conditional in a v2 if that's preferred, though.

Thomas

[gitgitgadget]

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

Tuomas Ahola <taahol@utu.fi> writes:

> "Thomas Bachem via GitGitGadget" <gitgitgadget@gmail.com> wrote:
>
>> +			warning(_("unrecognized value for '%s': '%.*s'"),
>> +				var, (int)(end - value), value);
>> +			value = end + !!*end;
>> +		}
>
> Reading CodingGuidelines, it seems that `!!` is a construct we would
> like to avoid:
>
>  - Some clever tricks, like using the !! operator with arithmetic
>    constructs, can be extremely confusing to others.  Avoid them,
>    unless there is a compelling reason to use them.
>
> --Tuomas

But here it is compelling.  You want to normalize random byte into 0
or 1, depending on "is it 0 or non-zero?" criteria, and it certainly
is easier to read than (*end ? 1 : 0).

Quite honestly, I do not think !! qualifies as "clever trick that is
confusing" anymore.  Perhaps putting the rule there is misguided.

Thanks.

[gitgitgadget]

Tuomas Ahola wrote on the Git mailing list (how to reply to this email):

Junio C Hamano <gitster@pobox.com> wrote:

> Tuomas Ahola <taahol@utu.fi> writes:
> 
> > "Thomas Bachem via GitGitGadget" <gitgitgadget@gmail.com> wrote:
> >
> >> +			warning(_("unrecognized value for '%s': '%.*s'"),
> >> +				var, (int)(end - value), value);
> >> +			value = end + !!*end;
> >> +		}
> >
> > Reading CodingGuidelines, it seems that `!!` is a construct we would
> > like to avoid:
> >
> >  - Some clever tricks, like using the !! operator with arithmetic
> >    constructs, can be extremely confusing to others.  Avoid them,
> >    unless there is a compelling reason to use them.
> >
> > --Tuomas
> 
> But here it is compelling.  You want to normalize random byte into 0
> or 1, depending on "is it 0 or non-zero?" criteria, and it certainly
> is easier to read than (*end ? 1 : 0).
> 

Uh, of course.  Sorry for my mental short-circuit.

> Quite honestly, I do not think !! qualifies as "clever trick that is
> confusing" anymore.  Perhaps putting the rule there is misguided.
> 
> Thanks.

Okay.  I'll consider writing a patch to drop that from CodingGuidelines.

[gitgitgadget]

This patch series was integrated into seen via git@0a4a65c.

[gitgitgadget]

This branch is now known as tb/sideband-skip-unrecognized-allow-control-characters.

[gitgitgadget]

There was a status update in the "New Topics" section about the branch tb/sideband-skip-unrecognized-allow-control-characters on the Git mailing list:

The configuration parser for 'sideband.allowControlCharacters' has
been updated to correctly advance past unrecognized values,
preventing an infinite loop when an unknown control character
keyword is encountered.

Needs review.
cf. <xmqq8q3xh98n.fsf@gitster.g>
source: <pull.2261.git.1792154374653.gitgitgadget@gmail.com>

[gitgitgadget]

There was a status update in the "Cooking" section about the branch tb/sideband-skip-unrecognized-allow-control-characters on the Git mailing list:

The configuration parser for 'sideband.allowControlCharacters' has
been updated to correctly advance past unrecognized values,
preventing an infinite loop when an unknown control character
keyword is encountered.

Needs review.
cf. <xmqq8q3xh98n.fsf@gitster.g>
source: <pull.2261.git.1792154374653.gitgitgadget@gmail.com>