perltidy

[Benton, Kevin]

> On Tue, 2006-08-01 at 16:59 -0700, Benton, Kevin wrote:
> > I'm wondering how others in this list feel about changing our
entrance
> > criteria for new patches to include that the submitting developer
> > should run perltidy on the code prior to submitting it.
> 
> 	It's not such a bad idea.
> 
> 	I just ran perltidy on some of our existing files, though, and I
> don't
> totally like the output in some cases. Of course, I ran a version that
> must be older than the version you ran, because my version said that
> some of the specified switches were invalid. (The latest version
> available for Fedora Extras is 20031021.)

To get the latest, simply do...

cpan Perl::Tidy

and that will bring you up-to-date with 20060719.  Those extra switches
make a big difference.  You may want to suggest adding Perl::Tidy to
base FC support since it's such valuable tool.  If nothing else, FC
developers ought to consider upgrading to a much more current version
since 20031021 is nearly three years old.

> 	For example, try running it on Bugzilla/Bug.pm and see what it
> outputs.
> In a lot of cases, it really does improve the formatting. It also
makes
> a few mistakes in places, or changes the formatting in a way I don't
> think is great. I'm sure these can be changed by the appropriate
command
> line, but I don't know them:
> 
> 	* It adds spaces after parentheses, where we normally don't.

That's the -pt option.

       The -pt=n or --paren-tightness=n parameter controls the space
       within parens.  The example below shows the effect of the three
       possible values, 0, 1, and 2:

        if ( ( my $len_tab = length( $tabstr ) ) > 0 ) {  # -pt=0
        if ( ( my $len_tab = length($tabstr) ) > 0 ) {    # -pt=1
(default)
        if ((my $len_tab = length($tabstr)) > 0) {        # -pt=2

       When n is 0, there is always a space to the right of a '(' and to
       the left of a ')'.  For n=2 there is never a space.  For n=1, the
       default, there is a space unless the quantity within the parens
is
       a single token, such as an identifier or quoted string.

...

> 	* It adds comments like "# end unless (blah)" even to really
short
> blocks. It's nice for long blocks, but it's unnecessary for subs and
> short blocks.

To set the length for ## end if (blah) blocks, that's -csci with a
default of six lines.

       -csci=n, or --closing-side-comment-interval=n
           where "n" is the minimum number of lines that a block must
have
           in order for a closing side comment to be added.  The default
           value is "n=6".  To illustrate:

                   # perltidy -csci=2 -csc
                   sub message {
                       if ( !defined( $_[0] ) ) {
                           print("Hello, World\n");
                       } ## end if ( !defined( $_[0] ))
                       else {
                           print( $_[0], "\n" );
                       } ## end else [ if ( !defined( $_[0] ))
                   } ## end sub message

           Now the "if" and "else" blocks are commented.  However, now
           this has become very cluttered.

> 	* It reformats some $dbh-> statements in a really strange way.
:-)

   -fs,  --format-skipping
       This flag, which is enabled by default, causes any code between
       special beginning and ending comment markers to be passed to the
       output without formatting.  The default beginning marker is #<<<
       and the default ending marker is #>>> but they may be changed
(see
       next items below).  Additional text may appear on these special
       comment lines provided that it is separated from the marker by at
       least one space.  For example

        #<<<  do not let perltidy touch this
           my @list = (1,
                       1, 1,
                       1, 2, 1,
                       1, 3, 3, 1,
                       1, 4, 6, 4, 1,);
        #>>>

       The comment markers may be placed at any location that a block
com-
       ment may appear.  If they do not appear to be working, use the
-log
       flag and examine the .LOG file.  Use -nfs to disable this
feature.

> 	* It violates perlstyle in that it puts the ending brace of a
> multi-line "if" statement on the end of the statement, and not on the
> next line aligned with the closing brace.

I'm trying to visualize what you're saying - a multi-line if meaning
that the conditionals take up more than one line or that the statements
after take up more than one line or both?  I'm basing my style choices
primarily on Conway's Perl Best Practices with a few variances.

> 	It does a lot of other good reformatting, though. I picked
> Bugzilla/Bug.pm on purpose because it has a lot of old code, and it
> improved parts of it.

I agree.  I picked buglist.cgi because it's just plain hairy.

As Conway put it, I think that once we come up with a fixed set of
params for perltidy, a lot of the style arguments will "go away" because
we simply follow one standard and use software to enforce it.  I think
it'll also make reviewing code a lot easier because it'll be relatively
easy to see if someone is following our style guidelines.  It'll also
put an emphasis on operations making it easier to spot bugs.

For example...

if ($somevar
    + $someothervar
    * $yetanothervar
    / 3)

... emphasizes the operations being done, making it easier to spot bugs.

This configuration also enforces a max line length preventing users from
creating exceedingly long lines.  That also helps reviews go faster.
    
---
Kevin Benton
Perl/Bugzilla Developer/Administrator, Perforce SCM Administrator
AMD - ECSD Software Validation and Tools
 
The opinions stated in this communication do not necessarily reflect the
view of Advanced Micro Devices and have not been reviewed by management.
This communication may contain sensitive and/or confidential and/or
proprietary information.  Distribution of such information is strictly
prohibited without prior consent of Advanced Micro Devices.  This
communication is for the intended recipient(s) only.  If you have
received this communication in error, please notify the sender, then
destroy any remaining copies of this communication.