You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by David James <dj...@collab.net> on 2006/04/13 16:59:41 UTC

Re: svn commit: r19341 - trunk/tools/dev

On 4/13/06, maxb@tigris.org <ma...@tigris.org> wrote:
>      name_components = name.split()
>      if len(name_components) == 1:
> -      if name_components[0] != name:
> -        complain("Python couldn't have done that, could it?", True)
> +      name = name_components[0] # Effectively, name = name.strip()

Effectively, you've just written:
   name_components = name.split()
   if len(name_components) == 1:
      name = name.strip()
      ...

Why only strip "name" if len(name_components) == 1? This isn't
correct. Here's an example test case:
   name = "Max Bowsher <ma...@bowsher.com> "

Your name has more than one component, but it still needs to be
stripped, so that we can correctly detect that name_components[-1][-1]
== '>'. I think we'd be better off to apply Madan's patch, which
correctly adds the "name.strip()" call before the split.

Cheers,

David



-
David James -- http://www.cs.toronto.edu/~james

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn commit: r19341 - trunk/tools/dev

Posted by "C. Michael Pilato" <cm...@collab.net>.
Max Bowsher wrote:
> David James wrote:
> 
>>>On 4/13/06, maxb@tigris.org <ma...@tigris.org> wrote:
>>>
>>>>     name_components = name.split()
>>>>     if len(name_components) == 1:
>>>>-      if name_components[0] != name:
>>>>-        complain("Python couldn't have done that, could it?", True)
>>>>+      name = name_components[0] # Effectively, name = name.strip()
>>>
>>>Effectively, you've just written:
>>>   name_components = name.split()
>>>   if len(name_components) == 1:
>>>      name = name.strip()
>>>      ...
>>>
>>>Why only strip "name" if len(name_components) == 1? This isn't
>>>correct. Here's an example test case:
>>>   name = "Max Bowsher <ma...@bowsher.com> "
>>>
>>>Your name has more than one component, but it still needs to be
>>>stripped, so that we can correctly detect that name_components[-1][-1]
>>>== '>'. I think we'd be better off to apply Madan's patch, which
>>>correctly adds the "name.strip()" call before the split.
> 
> 
> No, for two reasons:
> 
> 1) 'name' is not referenced at all in the 'else:' block.
> 
> 2) split() will eat the whitespace anyway, no need for an initial
>    strip().

Interesting.  I would have expected "foo bar ".split to result in ["foo",
"bar", ""], but it doesn't.  Weird.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

RE: svn commit: r19341 - trunk/tools/dev

Posted by Madan U S <ma...@collab.net>.
Comments inline...

-----Original Message-----
From: Max Bowsher [mailto:maxb1@ukf.net]
Sent: Thu 4/13/2006 10:41 PM
To: David James
Cc: dev@subversion.tigris.org; maxb@tigris.org; Madan U S
Subject: Re: svn commit: r19341 - trunk/tools/dev
 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

David James wrote:
> On 4/13/06, maxb@tigris.org <ma...@tigris.org> wrote:
>>      name_components = name.split()
>>      if len(name_components) == 1:
>> -      if name_components[0] != name:
>> -        complain("Python couldn't have done that, could it?", True)
>> +      name = name_components[0] # Effectively, name = name.strip()
> 
> Effectively, you've just written:
>    name_components = name.split()
>    if len(name_components) == 1:
>       name = name.strip()
>       ...
> 
> Why only strip "name" if len(name_components) == 1? This isn't
> correct. Here's an example test case:
>    name = "Max Bowsher <ma...@bowsher.com> "
> 
> Your name has more than one component, but it still needs to be
> stripped, so that we can correctly detect that name_components[-1][-1]
> == '>'. I think we'd be better off to apply Madan's patch, which
> correctly adds the "name.strip()" call before the split.

No, for two reasons:

1) 'name' is not referenced at all in the 'else:' block.

2) split() will eat the whitespace anyway, no need for an initial
   strip().

Madan : sure, split will eat the whitespace, but in the line...
if name_components[0] != name:
the RHS is the 'non-split' variable... which needs to be striped to be compared.

Also, its one thing to change the troublesome log... I would give some space to human error there... writing 'zhakov ' instead of 'zhakov' is not a huge mistake and *could* be permitted...

Regards,
Madan.

Re: svn commit: r19341 - trunk/tools/dev

Posted by Max Bowsher <ma...@ukf.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

David James wrote:
> On 4/13/06, maxb@tigris.org <ma...@tigris.org> wrote:
>>      name_components = name.split()
>>      if len(name_components) == 1:
>> -      if name_components[0] != name:
>> -        complain("Python couldn't have done that, could it?", True)
>> +      name = name_components[0] # Effectively, name = name.strip()
> 
> Effectively, you've just written:
>    name_components = name.split()
>    if len(name_components) == 1:
>       name = name.strip()
>       ...
> 
> Why only strip "name" if len(name_components) == 1? This isn't
> correct. Here's an example test case:
>    name = "Max Bowsher <ma...@bowsher.com> "
> 
> Your name has more than one component, but it still needs to be
> stripped, so that we can correctly detect that name_components[-1][-1]
> == '>'. I think we'd be better off to apply Madan's patch, which
> correctly adds the "name.strip()" call before the split.

No, for two reasons:

1) 'name' is not referenced at all in the 'else:' block.

2) split() will eat the whitespace anyway, no need for an initial
   strip().


Max.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (Cygwin)

iD8DBQFEPoYyfFNSmcDyxYARAlr+AJ0fdsWXvW1qZJtEfz2uMrhRM9ITmwCg1WG1
sArv1SmVISOMa5HJFhVU//w=
=8O0R
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org