Skip Menu |

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the Perl-Critic CPAN distribution.

Report information
The Basics
Id: 79162
Status: open
Priority: 0/
Queue: Perl-Critic
People
Owner: Nobody in particular
Requestors: jffry [...] posteo.net
Cc:
AdminCc:
Bug Information
Severity: Wishlist
Broken in: (no value)
Fixed in: (no value)

History Show all quoted text
  Thu Aug 23 08:02:28 2012 jffry [...] posteo.net - Ticket created
Subject: Mechanism to ignore .tdy files
Date: Thu, 23 Aug 2012 14:02:15 +0200
To: bug-Perl-Critic [...] rt.cpan.org
From: Jeffrey Ratcliffe <jeffrey.ratcliffe [...] gmail.com>
I use P::C in conjunction with Perl::Tidy, which leaves .tdy files around. It would be convenient if there was a mechanism in all_critic_ok() for ignoring these .tdy files. An alternative would be if all_critic_ok() just considered files in MANIFEST.
  Thu Aug 23 18:30:04 2012 wyant [...] cpan.org - Correspondence added
On Thu Aug 23 08:02:28 2012, RATCLIFFE wrote: Show quoted text
> I use P::C in conjunction with Perl::Tidy, which leaves .tdy files > around. > > It would be convenient if there was a mechanism in all_critic_ok() for > ignoring these .tdy files. > > An alternative would be if all_critic_ok() just considered files in > MANIFEST.
Making all_critic_ok() work this way would be a change in its documented behavior, and I can't see doing that. Maybe a separate manifest_critic_ok() subroutine that takes one or more manifest files as arguments, defaulting to 'MANIFEST'. Tom
  Thu Aug 23 18:30:06 2012 The RT System itself - Status changed from 'new' to 'open'
  Thu Aug 23 18:30:23 2012 wyant [...] cpan.org - Severity Wishlist added
  Thu Aug 23 19:00:31 2012 jeff [...] imaginative-software.com - Correspondence added
Subject: Re: [rt.cpan.org #79162] Mechanism to ignore .tdy files
Date: Thu, 23 Aug 2012 16:00:17 -0700
To: bug-Perl-Critic [...] rt.cpan.org
From: Jeffrey Thalhammer <jeff [...] imaginative-software.com>
On Aug 23, 2012, at 3:30 PM, Tom Wyant via RT wrote: Show quoted text
> Making all_critic_ok() work this way would be a change in its documented > behavior, and I can't see doing that.
I think we can get away with it. As I understand the code now, anything with a .pl .pm or .PL extension is considered a perl file, as well as anything with 'perl" in the shebang line. That last one is mostly meant to catch executables that typically don't have an extension, and is probably more liberal than it needs to be. I suspect this is what is causing the problem. Rather than catching anything with a perl shebang, I think it would be reasonable to automatically exclude files with a .tdy extension. We already exclude things that look like backup files, so this isn't much different. It does change the behavior, but not in a way that seems radical to me. Tom -- do you see broader implications that I don't? Show quoted text
> Maybe a separate > manifest_critic_ok() subroutine that takes one or more manifest files as > arguments, defaulting to 'MANIFEST'.
That would be fine with me too, but not high on my priority list. Patches welcome. Actually, I'd be happy to give you a commit bit. -Jeff
  Thu Aug 23 22:34:11 2012 wyant [...] cpan.org - Correspondence added
On Thu Aug 23 19:00:31 2012, jeff@imaginative-software.com wrote: Show quoted text
> > On Aug 23, 2012, at 3:30 PM, Tom Wyant via RT wrote: >
> > Making all_critic_ok() work this way would be a change in its
> documented
> > behavior, and I can't see doing that.
> > I think we can get away with it. As I understand the code now, > anything with a .pl .pm or .PL extension is considered a perl file, > as well as anything with 'perl" in the shebang line. That last one > is mostly meant to catch executables that typically don't have an > extension, and is probably more liberal than it needs to be. I > suspect this is what is causing the problem.
I came to the same conclusion. Show quoted text
> > Rather than catching anything with a perl shebang, I think it would be > reasonable to automatically exclude files with a .tdy extension. We > already exclude things that look like backup files, so this isn't > much different. It does change the behavior, but not in a way that > seems radical to me. > > Tom -- do you see broader implications that I don't?
No, I was just being paranoid. I thought I'd see if one of the other developers came up with the "add '.tdy' files to the 'skip backup files' logic" solution, as a check on my own sanity. If we do it this way it's a one-liner. Show quoted text
>
> > Maybe a separate > > manifest_critic_ok() subroutine that takes one or more manifest
> files as
> > arguments, defaulting to 'MANIFEST'.
> > That would be fine with me too, but not high on my priority list.
Yeah. I suggested it because I have done a couple tools for myself that run off the manifest, because I tend to collect cruft. But if skipping .tdy files passes muster, my vote is to defer manifest_critic_ok() until it's really needed. Tom