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: 41715
Status: open
Priority: 0/
Queue: Perl-Critic
People
Owner: Nobody in particular
Requestors: EDAVIS [...] cpan.org
Cc:
AdminCc:
Bug Information
Severity: (no value)
Broken in: 1.092
Fixed in: (no value)
Attachments
patch

History Show all quoted text
  Mon Dec 15 07:59:53 2008 EDAVIS [...] cpan.org - Ticket created
Subject: CodeLayout::RequireConsistentNewlines omits filename
The policy CodeLayout::RequireConsistentNewlines doesn't report the filename that was being checked. For example, % perlcritic --verbose "%f:%l:%c:%m [%p] (%e)\n" some_file :15:12:Use the same newline through the source [CodeLayout::RequireConsistentNewlines] (Change your newlines to be the same throughout) I guess this is somehow related to the policy not using PPI. But it should still print the name of the file for %f.
  Mon Dec 15 09:02:34 2008 perl [...] galumph.com - Correspondence added
Subject: Re: [rt.cpan.org #41715] CodeLayout::RequireConsistentNewlines omits filename
Date: Mon, 15 Dec 2008 08:02:13 -0600
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
EDAVIS via RT wrote: Show quoted text
> The policy CodeLayout::RequireConsistentNewlines doesn't report the > filename that was being checked.
Ah. I see it does a bit of evil by simulating PPI guts. The proper thing for the policy to do is search the DOM for the whitespace element corresponding to the line ending.
  Mon Dec 15 09:02:35 2008 The RT System itself - Status changed from 'new' to 'open'
  Thu Jan 15 17:23:34 2009 mgrimes [...] cpan.org - Correspondence added
On Mon Dec 15 09:02:34 2008, clonezone wrote: Show quoted text
> Ah. I see it does a bit of evil by simulating PPI guts. The proper > thing for the policy to do is search the DOM for the whitespace > element corresponding to the line ending.
It appears that PPI is converting line endings for us. By the time we could walk the DOM, all the line endings are consistent. Either this should be reported as a bug in PPI, or we can fix this with the simple but ugly hack of setting the $violation->{_filename} directly. A patch is attached.
Download patch
application/octet-stream 1k

Message body not shown because it is not plain text.

  Fri Jan 16 14:46:50 2009 mgrimes [...] cpan.org - Correspondence added
The patch has been applied to a branch in the svn repos: http://perlcritic.tigris.org/svn/perlcritic/branches/rt41715 -Mark
  Sat Jan 17 15:30:26 2009 perl [...] galumph.com - Correspondence added
Subject: Re: [rt.cpan.org #41715] CodeLayout::RequireConsistentNewlines omits filename
Date: Sat, 17 Jan 2009 14:29:32 -0600
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
Mark Grimes via RT wrote: Show quoted text
> The patch has been applied to a branch in the svn repos:
How about modifying the Violation constructor to take a filename parameter rather than violating encapsulation? The question is whether it's time to switch to named parameters for that constructor.
  Sun Jan 18 21:45:51 2009 thaljef [...] cpan.org - Correspondence added
On Sat Jan 17 15:30:26 2009, clonezone wrote: Show quoted text
> Mark Grimes via RT wrote:
> > The patch has been applied to a branch in the svn repos:
> > How about modifying the Violation constructor to take a filename > parameter rather than violating encapsulation? The question is > whether it's time to switch to named parameters for that constructor.
I prefer this idea over adding a getter/setter for the filename attribute. Elliot has taught me to appreciate immutable objects. I regret that I didn't use named parameters in all the interfaces. If we can do it in a backward compatible way, I definitely support adding named parameters on the Violation constructor.
  Mon Jan 19 09:52:54 2009 perl [...] galumph.com - Correspondence added
Subject: Re: [rt.cpan.org #41715] CodeLayout::RequireConsistentNewlines omits filename
Date: Mon, 19 Jan 2009 08:52:35 -0600
To: bug-Perl-Critic [...] rt.cpan.org
From: Elliot Shank <perl [...] galumph.com>
Jeffrey Ryan Thalhammer via RT wrote: Show quoted text
> I regret that I didn't use named parameters in all the interfaces. If > we can do it in a backward compatible way, I definitely support adding > named parameters on the Violation constructor.
There's a couple of different options: add a fourth parameter which is a hash reference or look for a single parameter which is a hash reference which would contain all values.