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)



Subject: CodeLayout::RequireConsistentNewlines omits filename
Download (untitled) / with headers
text/plain 428b
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.
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>
Download (untitled) / with headers
text/plain 302b
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.
Download (untitled) / with headers
text/plain 525b
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.

Download (untitled) / with headers
text/plain 125b
The patch has been applied to a branch in the svn repos: http://perlcritic.tigris.org/svn/perlcritic/branches/rt41715 -Mark
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>
Download (untitled) / with headers
text/plain 285b
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.
Download (untitled) / with headers
text/plain 663b
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.
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>
Download (untitled) / with headers
text/plain 411b
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.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.