There was a little bit of discussion on irc a few days back about perltidy and using it to get a bit of consistency in the formatting of the codebase. (consistency is good it means the surproses are more likely to be good than unpleasant). For those of you unfamiliar with perltidy its the program in the perl module Perl::Tidy which if you run it on a perl script produces a version consistently formatted according to irs rules. For more info see the docs at http://perltidy.sourceforge.net also man perltidy and perldoc Perl::Tidy The default rules in perltidy are an expansion of those in the perl style guide (perldoc perlstyle). Koha has its own set of recommendations, for the most part these are the defaults with perltidy. The uncommented lines in xt/perltidyrc show where we've overridden the defaults, they are: cuddled_else opening-brace-always-on-tight (a logical extension of cuddling your else) paren-tightness=1 (uncommented as if a non-default but is the perltidy default) vertical-tightness ( see the examples in Perl Best Practices for a reason for this) maximum-line-length=178 I would suggest that the maximum-line-length be reduced ( The default value is 80), my reasoning is this: although we all have editors and terminals that handle long lines, we actually like to use that screen estate to see more than one file, so that letting the code wander off to the right makes it easier to miss important clues when reading it. Especially when using tools like vimdiff, git diff, git blame etc. I feel if you have a line like: ($lots, $of, $returns) = call_subroutine( $with, $more, $arguments, $than, $it $should @have) if ($i_want_to_hide_my_conditional && $x == $y); a few judiciously placed new lines make it more readable and more maintainable, and with a long line length they disappear. Also a quick unscientific scan through the existing codebase a few days back revealed 96.96% of existing lines are 80 characters or less ( 99.7% are less than 100) so that perltidying with a long line length may have a detrimental effect in some cases. Also it seems we are by default using shorter lines. I think we should resolve this before considering any cleanup operations. Finally a tip. I found this in the book Perl Hacks with these two vim mappings: map ,pt <ESC>:%! perltidy<CR> map ,ptv <ESC>: '<,'>! perltidy<CR> I can use the commands ,pt and ,ptv in vim tp perltidy the file, or a region of the file within the editor. The latter is esprcially handy if I've added a few lines to a file and I want them formatted consistently but I don't want to obscure the files revision history by cleaning up code I've not touched. Colin -- Colin Campbell Chief Software Engineer, PTFS Europe Limited Content Management and Library Solutions +44 (0) 208 366 1295 (phone) +44 (0) 7759 633626 (mobile) colin.campbell@ptfs-europe.com skype: colin_campbell2 http://www.ptfs-europe.com
Hi, On Fri, Feb 12, 2010 at 10:51 AM, Colin Campbell <colin.campbell@ptfs-europe.com> wrote:
Also a quick unscientific scan through the existing codebase a few days back revealed 96.96% of existing lines are 80 characters or less ( 99.7% are less than 100) so that perltidying with a long line length may have a detrimental effect in some cases. Also it seems we are by default using shorter lines.
I think 132 is a good compromise value - 80 is too short, IMO.
I think we should resolve this before considering any cleanup operations.
Cleanup should be done with some care, however. First, although it's not common at all for perltidy to break code, it can happen - whitespace-only patches need to be tested the same as any other. More importantly, it is essential that whitespace changes be separated from bugfixes and functionality patches. In other words, please don't make a change, then run perltidy on the files you touched, then commit - the resulting patch will likely have a lot of diff lines that are extraneous to the bugfix itself. Instead, please make your change, then commit, then commit the perltidy change (or vice versa). I'm in favor of adopting consistent formatting of the code. I would even go so far to suggest that we move towards requiring frequent contributors to run new work through perltidy before submitting it. However, if there's any inclination to do a mass reformatting of existing code, I think there's only one not-bad time in the near future to do it - at the very beginning of 3.4. Unfortunately, there's never going to be a painless time to do a global cleanup: doing it now will make it more difficult to process patches and stray kittens for 3.2, while doing it at the beginning of 3.4 will make it more difficult to backport bugfixes into the 3.2 maintenance branch. Regards, Galen -- Galen Charlton gmcharlt@gmail.com
Finally a tip. I found this in the book Perl Hacks with these two vim mappings: map ,pt <ESC>:%! perltidy<CR> map ,ptv <ESC>: '<,'>! perltidy<CR>
You can also filter pass without mappings. Just select the text you want (* V* to enter visual mode, arrow up/down to select), then *!perltidy* to reformat the selection. Very useful however you do it! --Joe
participants (3)
-
Colin Campbell -
Galen Charlton -
Joe Atzberger