mlawren / githook-perltidy Goto Github PK

Occasionally data paragraphs in PODs are mangled during a tidying run. This is due to how the POD::Tidy module parses and cleans PODs in the codebase, but since there is no way to change or alter this behavior from githook-perltidy, I thought it was worth filing a bug on this project as well.

Long story short: Pod::Tidy should be ignoring everything in between =begin and =end, (data paragraphs) and leaving them as-is instead of wrapping them, but it isn't.

AFAICT it isn't even looking at or detecting the existence of of those format blocks. I went through the code of Pod::Tidy (which subclasses Pod::Wrap which subclasses Pod::Parser) and in all three of those modules I couldn't find any special cases for data paragraphs. It seems like it should be the responsibility of Pod::Wrap and Pod::Tidy to detect that a paragraph of text is a data paragraph and not perform any tidying on it. They are not examining the parse tree or anything else beyond whether a paragraph is a verbatim or a textblock. So what is happening is it is seeing chunks of text as textblocks (not double-checking to see if they are data paragraphs) and wraps them.

We discovered this because we had sample pieces of JSON in our own =begin and =end data blocks which were then reformatted during a tidy run.

http://perldoc.perl.org/perlpodspec.html#About-Data-Paragraphs-and-%22%3dbegin%2f%3dend%22-Regions

"Data paragraphs are typically used for inlining non-Pod data that is to be used (typically passed through) when rendering the document to a specific format"

Every time I have a merge with conflicts githook-perltidy "breaks" the merge commit message. this is what appear after fixing the conflicts and trying to commit the merge:

  1 # It looks like you may be committing a merge.
  2 # If this is not correct, please remove the file
  3 # .git/MERGE_HEAD
  4 # and try again.
  5 
  6 
  7 # Please enter the commit message for your changes. Lines starting
  8 # with '#' will be ignored, and an empty message aborts the commit.

and on closing the editor:

fatal: could not open '.git/MERGE_HEAD' for reading: No such file or directory

It seems you have hardcoded the recognized extensions (here, for example). It would be nice if those could be passed in as a command-line option, so that, for example, I could add ".cgi".

Hello!

I have a setup script at $WORK that installs this githook. One of my coworkers ran it last week, and it ran into the issue where it fails against the new release of OptArg2. I see you're the owner of both packages, and it looks like you've fixed it already in master. Do you have any idea when you're going to release an updated version of the githook?

Thanks for all your work, eh. I've gotten a lot of value out of this tool. 😀

I've install githook-perltidy, and installed it in one of my gits.

The Perl is installed using mac-port, that install perl itself in /opt/local/bin/perl, and when a CPAN module install a script it is done inside /opt/local/libexec/perl5.XX/sitebin/
(with XX stand for the Perl version)
This is not necessarily inside the PATH. (I work with multiple Perl-s, and each have its own installation)

Inside the pre/post-commit file, githook is simply writing "githook-perltidy pre-commit". which fails on my machine.

Is it possible to use the same path that was used to run the install command?
If someone ran "githook-perltidy install" then use the same.
But if someone ran "/opt/local/libexec/perl5.12/sitebin/githook-perltidy install" then use this path.

Does it make sense?
Shmuel.

Hi I use your tool and find it very useful.

Though, because I use auto-generated dbicdump classes, I have a problem because the top part of those files are not supposed to be modified therefore formatted.

I wish it would be possible to define a --ignore-path option so perltidy would just don't process this path when the hook is launched.

Any idea on how to do this or is it a feature idea?

Thanks.

Running on MacOSX and Perl 5.12.4

Running make test
PERL_DL_NONLAZY=1 /opt/local/bin/perl5.12 "-MExtUtils::Command::MM" "-e" "test_harness(0, 'inc', 'blib/lib', 'blib/arch')" t/*.t
t/githook-perltidy.t .. 2/? 
#   Failed test 'detect no-extension'
#   at t/githook-perltidy.t line 71.
#          got: '#!/opt/local/bin/perl5.12
# if (1) {
# print "dent\n";
# }
# '
#     expected: '#!/opt/local/bin/perl5.12
# if (1) {
#     print "dent\n";
# }
# '
githook-perltidy pre-commit:
    # saving non-indexed changes and tidying
    git stash save --quiet --keep-index githook-perltidy 2012-11-02T14:30:12
    git checkout-index -a
Skipping podtidy calls: no .podtidy-opts file at /Users/sfomberg/.cpan/build/App-githook-perltidy-0.10.2-1zKQkk/t/../blib/script/githook-perltidy line 164, <$fh> line 1.
perltidy file.pl
## Please see file file.pl.ERR
Use of uninitialized value $input_line_number in concatenation (.) or string at /opt/local/lib/perl5/site_perl/5.12.3/Perl/Tidy.pm line 4019.
:       Syntax check deactivated for safety; you shouldn't run this as root

githook-perltidy: pre-commit FAIL! Restoring...
    git reset --hard
HEAD is now at b88394b add file
    git stash pop --quiet --index
Command exited with value 1 at t/githook-perltidy.t line 75.
# Tests were run but no plan was declared and done_testing() was not seen.
t/githook-perltidy.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 1/6 subtests 

Test Summary Report
-------------------
t/githook-perltidy.t (Wstat: 65280 Tests: 6 Failed: 1)
  Failed test:  6
  Non-zero exit status: 255
  Parse errors: No plan found in TAP output
Files=1, Tests=6,  2 wallclock secs ( 0.02 usr  0.01 sys +  1.30 cusr  0.37 csys =  1.70 CPU)
Result: FAIL
Failed 1/1 test programs. 1/6 subtests failed.

pre-commit prints a lot of messages to STDOUT by default. I'd prefer no output unless something went wrong.

It would be great if this was capable of running perlcritic the same way it does perltidy (i.e., only on files being committed). While this is not an immediate need, it would be very nice to have, and I'm actually open to implementing it myself, given a bit of guidance on how best to do it.

It would be great if this is supported by pre-commit. Can we make this happen?