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

2011-10-09 Thread 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/
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel

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

2011-10-08 Thread Sascha Silbe 
Excerpts from Daniel Drake's message of 2011-10-06 16:58:53 +0200:

 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.

Thanks for the patch!

Acked-By: Sascha Silbe si...@activitycentral.com

Sascha

-- 
http://sascha.silbe.org/
http://www.infra-silbe.de/


signature.asc
Description: PGP signature
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel

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

2011-10-06 Thread Daniel Drake 
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:
 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)
-- 
1.7.6.4

___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel

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

2011-10-06 Thread Frederick Grose 
On Thu, Oct 6, 2011 at 10:58 AM, Daniel Drake d...@laptop.org 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.


But try this:
foo = 0
if foo:
print 'foo is not None'
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel

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

2011-10-06 Thread Daniel Drake 
On Thu, Oct 6, 2011 at 6:13 PM, Frederick Grose fgr...@gmail.com wrote:
 But try this:
 foo = 0
 if foo:
     print 'foo is not None'

Yes, I'm aware that the boolean operator also has meaning for
numerical values. But that doesn't affect my patch and the context of
this code. Or am I missing something?

Thanks,
Daniel
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel

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

2011-10-06 Thread Frederick Grose 
On Thu, Oct 6, 2011 at 1:22 PM, Daniel Drake d...@laptop.org wrote:

 On Thu, Oct 6, 2011 at 6:13 PM, Frederick Grose fgr...@gmail.com wrote:
  But try this:
  foo = 0
  if foo:
  print 'foo is not None'

 Yes, I'm aware that the boolean operator also has meaning for
 numerical values. But that doesn't affect my patch and the context of
 this code. Or am I missing something?

 Thanks,
 Daniel


foo alone as a boolean operator is an equivalent length check,
 and also serves as a is not None check too.


No, you are not missing something, but the casual reader may want to take
note that if foo: serves only as a is not None or is zero check.

So if foo is not None: is a more robust check for that singular condition.

That's all, just an aside. Thanks.   --Fred
___
Sugar-devel mailing list
Sugar-devel@lists.sugarlabs.org
http://lists.sugarlabs.org/listinfo/sugar-devel