On Tue, Jun 30, 2015 at 3:35 AM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> On Mon, Jun 29, 2015 at 8:06 PM, Yann Ylavic <ylavic....@gmail.com> wrote:
>>
>> Maybe defining (naming) inherit_before tristate values would help:
>
>
> Not really...
>
>> + a->inherit_before = (over->inherit_before == INHERIT_ON
>> + || (over->inherit_before == INHERIT_UNSET
>> + && base->inherit_before == INHERIT_ON));
>> if (a->inherit_before) {
>
>
> This logic was convoluted
Agreed.
> and therefore resulted in in the old default
> behavior if the option wasn't explicitly set.
I can't see this though, over->inherit_before == over->inherit_before
== INHERIT_UNSET resulted in a->inherit_before = FALSE in trunk and
TRUE in 2.[24].x.
> See the most recent trunk
> commits for a more legible solution that follows the design pattern used
> throughout other core modules.
That's simpler indeed.
I previously overlooked that the merge were already against the merged
parents, so base->inherit_before will be unset iff it is unused so
far, hence I was wrong about the third level issue...
( I had to play with gdb to figure out though :)
>
> Your logic above fails to preserve the unset state, and fails to when
> consider base->inherit_before is explicitly off.
I can't see this either...
I still think my logic was working too, just like the change you did
regarding max_line_length_set in r1688341 (you don't preserve the
unset state there either).
> This is why it is
> preferred not to invent new wheels when good wheels exist, particularly if
> there will be a square side on the new wheel.
That I can agree with ;)
Btw, I added your proposed changes to the 2.4.x/2.2.x proposals.
Regards,
Yann.
