Discussion:
Trouble with the .bool_set:N property
p***@public.gmane.org
2014-05-21 22:14:41 UTC
Permalink
Hello all (especially Joseph, who might have to deal with this),

Based on the discussion at http://tex.stackexchange.com/questions/179180
there seems to be a problem with the definition of \__keys_bool_set:Nn.
Namely, it is not obvious that the following code should fail

\keys_define:nn { mymodule }
{
key-1 .choice: ,
key-1 / choice-a .bool_set:N = \l_mymodule_choice_a_bool,
key-1 / choice-b .bool_set:N = \l_mymodule_choice_b_bool
}
\keys_set:nn { mymodule } { key-1 = choice-a }

and yet it hangs because \__keys_bool_set:Nn actually creates a choice
key with the choices 'true' and 'false' under the hood.

I feel it would be more natural if the code above behaved in the same
way as the following:

\keys_define:nn { mymodule }
{
key-2 .bool_set:N = \l_mymodule_choice_a_bool
}
\keys_set:nn { mymodule } { key-2 }

In other words, the code that now hangs should just be setting the
boolean to 'true'. If \__keys_bool_set:Nn and \__keys_bool_set_inverse:Nn
were redefined along the lines of the code below, we'd get that result.

\cs_new_protected:Npn \__keys_bool_set:Nn #1#2
{
\bool_if_exist:NF #1 { \bool_new:N #1 }
\__keys_cmd_set:nn { \l_keys_path_tl }
{
\str_case_x:nnF { \l_keys_value_tl }
{
{ true } { \use:c { bool_ #2 set_true:N } #1 }
{ false } { \use:c { bool_ #2 set_false:N } #1 }
}
{
\cs_if_exist:cTF
{ \c__keys_code_root_tl \l_keys_path_tl / \l_keys_value_tl
}
{ \use:c { bool_ #2 set_true:N } #1 }
{
\__msg_kernel_error:nnx { kernel } { boolean-values-only
}
{ \l_keys_key_tl }
}
}
}
\__keys_default_set:n { true }
}
\cs_new_protected:Npn \__keys_bool_set_inverse:Nn #1#2
{
\bool_if_exist:NF #1 { \bool_new:N #1 }
\__keys_cmd_set:nn { \l_keys_path_tl }
{
\str_case_x:nnF { \l_keys_value_tl }
{
{ true } { \use:c { bool_ #2 set_false:N } #1 }
{ false } { \use:c { bool_ #2 set_true:N } #1 }
}
{
\cs_if_exist:cTF
{ \c__keys_code_root_tl \l_keys_path_tl / \l_keys_value_tl
}
{ \use:c { bool_ #2 set_false:N } #1 }
{
\__msg_kernel_error:nnx { kernel } { boolean-values-only
}
{ \l_keys_key_tl }
}
}
}
\__keys_default_set:n { true }
}

Finally, and a lower priority: it might also be useful to add the
properties .bool_set_true:N and .bool_set_false:N as well. For example,
they could work like this:

\cs_new_protected:Npn \__keys_bool_set_true:Nn #1#2
{
\bool_if_exist:NF #1 { \bool_new:N #1 }
\__keys_cmd_set:nn { \l_keys_path_tl }
{ \use:c { bool_ #2 set_true:N } #1 }
\__keys_value_requirement:n { forbidden }
}
\cs_generate_variant:Nn \__keys_bool_set_true:Nn { c }
\cs_new_protected:Npn \__keys_bool_set_false:Nn #1#2
{
\bool_if_exist:NF #1 { \bool_new:N #1 }
\__keys_cmd_set:nn { \l_keys_path_tl }
{ \use:c { bool_ #2 set_false:N } #1 }
\__keys_value_requirement:n { forbidden }
}
\cs_generate_variant:Nn \__keys_bool_set_false:Nn { c }
\cs_new_protected:cpn { \c__keys_props_root_tl .bool_set_true:N } #1
{ \__keys_bool_set_true:Nn #1 { } }
\cs_new_protected:cpn { \c__keys_props_root_tl .bool_set_true:c } #1
{ \__keys_bool_set_true:cn {#1} { } }
\cs_new_protected:cpn { \c__keys_props_root_tl .bool_gset_true:N } #1
{ \__keys_bool_set_true:Nn #1 { g } }
\cs_new_protected:cpn { \c__keys_props_root_tl .bool_gset_true:c } #1
{ \__keys_bool_set_true:cn {#1} { g } }
\cs_new_protected:cpn { \c__keys_props_root_tl .bool_set_false:N } #1
{ \__keys_bool_set_false:Nn #1 { } }
\cs_new_protected:cpn { \c__keys_props_root_tl .bool_set_false:c } #1
{ \__keys_bool_set_false:cn {#1} { } }
\cs_new_protected:cpn { \c__keys_props_root_tl .bool_gset_false:N } #1
{ \__keys_bool_set_false:Nn #1 { g } }
\cs_new_protected:cpn { \c__keys_props_root_tl .bool_gset_false:c } #1
{ \__keys_bool_set_false:cn {#1} { g } }

All the best,

Jura Pintar
Joseph Wright
2014-05-22 07:31:53 UTC
Permalink
Post by p***@public.gmane.org
Based on the discussion at http://tex.stackexchange.com/questions/179180
there seems to be a problem with the definition of \__keys_bool_set:Nn.
Namely, it is not obvious that the following code should fail
\keys_define:nn { mymodule }
{
key-1 .choice: ,
key-1 / choice-a .bool_set:N = \l_mymodule_choice_a_bool,
key-1 / choice-b .bool_set:N = \l_mymodule_choice_b_bool
}
\keys_set:nn { mymodule } { key-1 = choice-a }
and yet it hangs because \__keys_bool_set:Nn actually creates a choice
key with the choices 'true' and 'false' under the hood.
I feel it would be more natural if the code above behaved in the same
\keys_define:nn { mymodule }
{
key-2 .bool_set:N = \l_mymodule_choice_a_bool
}
\keys_set:nn { mymodule } { key-2 }
In other words, the code that now hangs should just be setting the
boolean to 'true'. If \__keys_bool_set:Nn and \__keys_bool_set_inverse:Nn
were redefined along the lines of the code below, we'd get that result.
It's a bit more complicated than that :-)

If you look at the code, any case where someone sets up a 'choice of
choices' will fail. That's not good, and so before any worries about
booleans specifically I'll fix this more general issue. My feeling is
that trying to set up a 'choice of choices' key is an error in the
definition rather than at point of use, so I'm minded to trap such cases
and issue a error message. Does that sound reasonable? For example, what
is something like

key-1 .choice: ,
key-1 / choice-a .choice: ,
key-1 / choice-a / value-1 .code = ...

actually going to achieve?

I have an approach in mind: probably will be checked in later today.
Post by p***@public.gmane.org
Finally, and a lower priority: it might also be useful to add the
properties .bool_set_true:N and .bool_set_false:N as well. For example,
One the specifics of boolean keys, I can see the point here about
set_true/set_false as they match 'normal' variable setting. On the other
hand, the original design here was to recognise that a boolean key is
somewhat 'special': it is a choice from a limited range of values.
Moreover, my thinking was that enforcing "true"/"false" as the values
avoided the possible need for people to allow for "on"/"off" or
"yes"/"no" (I've worried about this in the past). It was also meant to
indicate that a boolean key should be one that 'reads' as

<thing> = (true|false)

rather than

<thing> = (foo|bar)

as that seems less useful to the end user (if the options are more
complex than true/false then some other description is needed at the
documentation level anyway).
--
Joseph Wright
p***@public.gmane.org
2014-05-22 09:02:10 UTC
Permalink
Post by Joseph Wright
It's a bit more complicated than that :-)
It usually is!
Post by Joseph Wright
If you look at the code, any case where someone sets up a 'choice of
choices'
Post by Joseph Wright
will fail. That's not good, and so before any worries about booleans
specifically I'll fix this more general issue. My feeling is that trying
to set up a
Post by Joseph Wright
'choice of choices' key is an error in the definition rather than at point
of use,
Post by Joseph Wright
so I'm minded to trap such cases and issue a error message. Does that
sound
Post by Joseph Wright
reasonable? For example, what is something like
key-1 .choice: ,
key-1 / choice-a .choice: ,
key-1 / choice-a / value-1 .code = ...
actually going to achieve?
Yes, I'd noticed that it will fail (I actually mentioned it in passing in
the
tex.stackexchange discussion), but it didn't worry me so much precisely
because
I couldn't think of any reason why one would want to set up a case like
that.
Of course, it's definitely better if attempts to do so raised an error
instead
of getting stuck in a loop!
Post by Joseph Wright
One the specifics of boolean keys, I can see the point here about
set_true/set_false as they match 'normal' variable setting. On the other
hand, the original design here was to recognise that a boolean key is
somewhat 'special': it is a choice from a limited range of values.
Moreover, my thinking was that enforcing "true"/"false" as the values
avoided the possible need for people to allow for "on"/"off" or "yes"/"no"
(I've worried about this in the past). It was also meant to indicate that
a
Post by Joseph Wright
boolean key should be one that 'reads' as
<thing> = (true|false)
rather than
<thing> = (foo|bar)
as that seems less useful to the end user (if the options are more complex
than true/false then some other description is needed at the documentation
level anyway).
I see your point, but I can also imagine cases where at the user end it
might
feel more natural to have something like

\mymoduledoclevelsetup{ turnfooingon }
\mymodulecoclevelsetup{ turnfooingoff }

instead of

\mymoduledoclevelsetup{ fooing = true }
\mymoduledoclevelsetup{ fooing = false }

I guess the decision here is really between nudging package authors towards
uniformity and trusting them to develop a syntax that's best for their
target
users on their own (it's not exactly imposing uniformity, because a package
writer stubborn enough could always type the few extra characters to get
" turnfooingon .code:n = { \bool_set_true:N \l_mymodule_fooing_bool } ").

I tend to be a fan of uniformity myself, but I wouldn't go as far as to
claim
I have a worked out ideological position on it. :)

Best,

Jura
Bruno Le Floch
2014-05-30 00:05:41 UTC
Permalink
Post by p***@public.gmane.org
Post by Joseph Wright
It's a bit more complicated than that :-)
It usually is!
Post by Joseph Wright
If you look at the code, any case where someone sets up a 'choice of
choices'
Post by Joseph Wright
will fail. That's not good, and so before any worries about booleans
specifically I'll fix this more general issue. My feeling is that trying
to set up a
Post by Joseph Wright
'choice of choices' key is an error in the definition rather than at point
of use,
Post by Joseph Wright
so I'm minded to trap such cases and issue a error message. Does that
sound
Post by Joseph Wright
reasonable? For example, what is something like
key-1 .choice: ,
key-1 / choice-a .choice: ,
key-1 / choice-a / value-1 .code = ...
actually going to achieve?
Yes, I'd noticed that it will fail (I actually mentioned it in passing in
the
tex.stackexchange discussion), but it didn't worry me so much precisely
because
I couldn't think of any reason why one would want to set up a case like
that.
Of course, it's definitely better if attempts to do so raised an error
instead
of getting stuck in a loop!
Post by Joseph Wright
One the specifics of boolean keys, I can see the point here about
set_true/set_false as they match 'normal' variable setting. On the other
hand, the original design here was to recognise that a boolean key is
somewhat 'special': it is a choice from a limited range of values.
Moreover, my thinking was that enforcing "true"/"false" as the values
avoided the possible need for people to allow for "on"/"off" or "yes"/"no"
(I've worried about this in the past). It was also meant to indicate that
a
Post by Joseph Wright
boolean key should be one that 'reads' as
<thing> = (true|false)
rather than
<thing> = (foo|bar)
as that seems less useful to the end user (if the options are more complex
than true/false then some other description is needed at the
documentation
level anyway).
I see your point, but I can also imagine cases where at the user end it
might
feel more natural to have something like
\mymoduledoclevelsetup{ turnfooingon }
\mymodulecoclevelsetup{ turnfooingoff }
instead of
\mymoduledoclevelsetup{ fooing = true }
\mymoduledoclevelsetup{ fooing = false }
I guess the decision here is really between nudging package authors towards
uniformity and trusting them to develop a syntax that's best for their
target
users on their own (it's not exactly imposing uniformity, because a package
writer stubborn enough could always type the few extra characters to get
" turnfooingon .code:n = { \bool_set_true:N \l_mymodule_fooing_bool } ").
Wait, how is that not covered by .bool_set:N and .bool_set_inverse:N?
Doing " turnfooingon .bool_set:N = \l_mymodule_fooing_bool " and "
turnfooingoff .bool_set_inverse:N = \l_mymodule_fooing_bool " allows
the user to use the keys turnfooingon and turnfooingoff as you
describe. Well, it also allows the user to do weirder things like
"turnfooingoff=false" to turn fooing on.

Bruno
Jura Pintar
2014-05-30 00:46:50 UTC
Permalink
Post by Bruno Le Floch
Wait, how is that not covered by .bool_set:N and .bool_set_inverse:N?
Doing " turnfooingon .bool_set:N = \l_mymodule_fooing_bool " and "
turnfooingoff .bool_set_inverse:N = \l_mymodule_fooing_bool " allows the
user to use the keys turnfooingon and turnfooingoff as you describe. Well, it
also allows the user to do weirder things like "turnfooingoff=false" to turn
fooing on.
Bruno
You're totally right: it is. I think I let my mind wander a bit when I was
writing the example... What's actually at issue is whether constructions
along the lines of

\mymoduledoclevelsetup{ fooing = activate }
\mymoduledoclevelsetup{ fooing = deactivate }

should be tacitly encouraged by making

\keys_define:nn { mymodule }
{
fooing .choice: ,
fooing/ activate .bool_set_true:N = \l_mymodule_fooing_bool ,
fooing / deactivate .bool_set_false:N = \l_mymodule_fooing_bool
}

available out of the box, or whether it's better to leave it to package
authors to use .code:n or some such alternative if they want to get
this behaviour.

Best,

Jura
Bruno Le Floch
2014-05-30 00:51:58 UTC
Permalink
Post by Jura Pintar
Post by Bruno Le Floch
Wait, how is that not covered by .bool_set:N and .bool_set_inverse:N?
Doing " turnfooingon .bool_set:N = \l_mymodule_fooing_bool " and "
turnfooingoff .bool_set_inverse:N = \l_mymodule_fooing_bool " allows the
user to use the keys turnfooingon and turnfooingoff as you describe.
Well, it
also allows the user to do weirder things like "turnfooingoff=false" to turn
fooing on.
Bruno
You're totally right: it is. I think I let my mind wander a bit when I was
writing the example... What's actually at issue is whether constructions
along the lines of
\mymoduledoclevelsetup{ fooing = activate }
\mymoduledoclevelsetup{ fooing = deactivate }
should be tacitly encouraged by making
\keys_define:nn { mymodule }
{
fooing .choice: ,
fooing/ activate .bool_set_true:N = \l_mymodule_fooing_bool ,
fooing / deactivate .bool_set_false:N = \l_mymodule_fooing_bool
}
available out of the box, or whether it's better to leave it to package
authors to use .code:n or some such alternative if they want to get
this behaviour.
Best,
Jura
Ah, ok, I understand. And I don't have a preference either way.

Regards,
Bruno

Loading...