[Sugar-devel] [PATCH 1/2] Fix presence detection of keyboard layouts/options

[Simon Schampijer]

On 10/06/2011 04:58 PM, Daniel Drake wrote:
> "foo is []" is not a valid way of checking if a list is empty
> as this code intends.
>
> "foo" alone as a boolean operator is an equivalent length check,
> and also serves as a "is not None" check too.
> ---
>   bin/sugar-session |    5 ++---
>   1 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/bin/sugar-session b/bin/sugar-session
> index 4b714c3..a0ec14f 100755
> --- a/bin/sugar-session
> +++ b/bin/sugar-session
> @@ -147,8 +147,7 @@ def setup_keyboard_cb():
>               layouts_list.append(layout.split('(')[0])
>               variants_list.append(layout.split('(')[1][:-1])
>
> -        if layouts_list is not None and layouts_list is not [] \
> -                and variants_list is not None and variants_list is not []:
> +        if layouts_list and variants_list:

 From the pep8 this is the right way of checking if a list is empty [1]. 
Might be worth referencing in the commit message. "For sequences, 
(strings, lists, tuples), use the fact that empty sequences are false."

The check if the list is None is surprising as well actually, as the 
list should never be None.

>               configrec.set_layouts(layouts_list)
>               configrec.set_variants(variants_list)
>
> @@ -159,7 +158,7 @@ def setup_keyboard_cb():
>
>           options = gconf_client.get_list(\
>               '/desktop/sugar/peripherals/keyboard/options', gconf.VALUE_STRING)
> -        if options is not [] and options is not None:
> +        if options:
>               configrec.set_options(options)
>
>           configrec.activate(engine)

Patch looks good, acked as well by me.

Regards,
    Simon

[1] http://www.python.org/dev/peps/pep-0008/