[Python-Dev] Re: [Python-checkins] CVS: distutils/distutils/command sdist.py,1.25,1.26

[Greg Stein]

On Wed, Jun 07, 2000 at 05:08:17PM -0700, Greg Ward wrote:
> Update of /cvsroot/python/distutils/distutils/command
> In directory slayer.i.sourceforge.net:/tmp/cvs-serv15881
> 
> Modified Files:
> 	sdist.py 
> Log Message:
> Made all debug output go through the 'debug_print()' method instead of
> directly printing to stdout.  This was a bit more work than it sounds like
> it should have been:
>   * turned 'select_pattern()' and 'exclude_pattern()' from functions into
>     methods, so they can refer to 'self' to access the method

You wouldn't have to do this if you make debug_print() a function. There is
nothing in debug_print() that requires "self", so its existence as a method
is suspect in the first place. Considering the fallout changes, I'd say move
debug_print() to a plain function (in "utils").

Unless you were planning on more stuff in there; if so, then a comment in
debug_print would be nice.

>...
> ***************
> *** 297,301 ****
>               if (cur_dir == dir and
>                   (pattern_re is None or pattern_re.match (cur_base))):
> !                 print "removing %s" % self.files[i]
>                   del self.files[i]
>   
> --- 298,302 ----
>               if (cur_dir == dir and
>                   (pattern_re is None or pattern_re.match (cur_base))):
> !                 self.debug_print("removing %s" % self.files[i])
>                   del self.files[i]

Gah... "pattern_re.match (cur_base)" ...

You do realize that God intended there to be no spaces between a function
name and the opening paren? Heretic!

:-)

Really -- I have noticed this throughout a lot of the distutils code. That
space in there is really awful. It reduces the "binding" of the name to the
paren, which changes the meaning during a quick read. Put another way: when
you skim code, NAME( is a function call and NAME ( is something else. A
space after a name usually indicates the name is participating in an
expression, rather than being used as a function name. Then, you hit the
paren and go "what the heck? <pause> oh. a function call."

Please... can they be removed?

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/