[perl #57884] [PATCH] file_type option for Parrot::Configure::Compiler-genfile

2008-08-28 Thread James Keenan via RT
No complaints in five days.  No smolder test failures.  Marking ticket
resolved.


[perl #57884] [PATCH] file_type option for Parrot::Configure::Compiler-genfile

2008-08-24 Thread James Keenan via RT
On Sat Aug 23 06:21:57 2008, mmcleric wrote:

 
 Week is over and release has happened, can this be commited?
 I'm asking because I want to get familiar with submitting process and
 move on to sending new and more interesting patches :)
 

Applied in r30521.  I'm running coverage analysis to see whether there
are any new branches/conditions in lib/Parrot/Configure/Compiler.pm
which need testing.


[perl #57884] [PATCH] file_type option for Parrot::Configure::Compiler-genfile

2008-08-24 Thread James Keenan via RT
On Sun Aug 24 09:30:43 2008, [EMAIL PROTECTED] wrote:
   I'm running coverage analysis to see whether there
 are any new branches/conditions in lib/Parrot/Configure/Compiler.pm
 which need testing.


I never cease to find coverage analysis rewarding for its ability to
enable you to spot dead code.  After running Devel::Cover over the
configuration and build tools, I noticed that lines 315-317 of
lib/Parrot/Configure/Compiler.pm had one-half of the branch and
two-thirds of the condition uncovered by the test suite.

if ( !exists $options{file_type}  $target =~ m/makefile$/i ) {
$options{file_type} = 'makefile';
}

These lines were formerly found higher up inside genfile().   The more I
stared at them, the more I realized that they now duplicated code at
lines 298-301:

if ( !exists $options{file_type}) {
if ( $target =~ m/makefile$/i ) {
$options{file_type} = 'makefile';
}

So they could be excised.

I further realized that since with makefiles we always want
'replace_slashes' and 'conditioned_lines' to be enabled, we could
simplify this:

if ( $options{file_type} eq 'makefile' ) {
exists $options{replace_slashes}   or
$options{replace_slashes}   = 1;
exists $options{conditioned_lines} or
$options{conditioned_lines} = 1;
}

... to this:

if ( $options{file_type} eq 'makefile' ) {
$options{replace_slashes}   = 1;
$options{conditioned_lines} = 1;
}

... and in the process eliminate two conditions which were also coming
up short in coverage analysis.  Changes were committed in r30527.

kid51




[perl #57884] [PATCH] file_type option for Parrot::Configure::Compiler-genfile

2008-08-13 Thread via RT
# New Ticket Created by  Vyacheslav Matjukhin 
# Please include the string:  [perl #57884]
# in the subject line of all future correspondence about this issue. 
# URL: http://rt.perl.org/rt3/Ticket/Display.html?id=57884 


genfile() method at the Parrot::Configure::Compiler module generates wrong
vim syntax modelines, i.e., it prints ft=c always.
This patch fixes it by implementing new file_type option.
If file_type isn't specified, genfile() tries to automatically recognize
file type by looking at a target file name.
Also, understanding file_type allows method users to forget about a
comment_type option in most cases.
This obviously generalizes old makefile option, so i removed it.


Patch contents description follows.
(as required by How To Submit A Patch from submissions.pod; should i
really do this? It all seems pretty obvious...)

config/gen/config_h.pm, config/gen/crypto.pm, config/gen/makefiles.pm:
using new file_type option where needed, sometimes instead of makefile
option.

lib/Parrot/Configure/Compiler.pm:
file_type option implementation and pod; hash with description of possible
file_type's and corresponding comment_type's and vim_ft's.

t/configure/034-step.t:
trivial test updates.

PS. I should've refactored 034-step.t too, but this can be done next time.
There are no tests checking genfile() output, they all just check return
value...
Index: lib/Parrot/Configure/Compiler.pm
===
--- lib/Parrot/Configure/Compiler.pm	(revision 30189)
+++ lib/Parrot/Configure/Compiler.pm	(working copy)
@@ -34,6 +34,25 @@
 move_if_diff
 );
 
+our %file_types_info = (
+makefile = {
+comment_type= '#',
+vim_ft  = 'make',
+},
+c = {
+comment_type= '/*',
+vim_ft  = 'c',
+},
+pmc = {
+comment_type= '/*',
+vim_ft  = 'pmc',
+},
+perl = {
+comment_type= '#',
+vim_ft  = 'perl',
+},
+);
+
 =item Ccc_gen()
 
 $conf-cc_gen($source)
@@ -46,7 +65,7 @@
 my $conf   = shift;
 my $source = shift;
 
-$conf-genfile( $source, test_$$.c );
+$conf-genfile( $source, test_$$.c, file_type = 'none' );
 }
 
 =item Ccc_build()
@@ -179,13 +198,15 @@
 
 =over 4
 
-=item makefile
+=item file_type
 
-If set to a true value, this flag sets (unless overridden) Ccomment_type
-to '#', Creplace_slashes to enabled, and Cconditioned_lines to enabled.
+If set to a Cmakefile, Cc or Cperl value, Ccomment_type will be set
+to corresponding value.
+Moreover, when set to a Cmakefile value, it will set Creplace_slashes to
+enabled, and Cconditioned_lines to enabled.
 
-If the name of the file being generated ends in CMakefile, this option
-defaults to true.
+Its value will be detected automatically by target file name unless you set
+it to a special value Cnone.
 
 =item conditioned_lines
 
@@ -272,21 +293,52 @@
 open my $in,  '', $source   or die Can't open $source: $!;
 open my $out, '', $target.tmp or die Can't open $target.tmp: $!;
 
-if ( !exists $options{makefile}  $target =~ m/makefile$/i ) {
-$options{makefile} = 1;
+if ( !exists $options{file_type}) {
+if ( $target =~ m/makefile$/i ) {
+$options{file_type} = 'makefile';
+}
+elsif ($target =~ m/\.pl$/i ) {
+$options{file_type} = 'perl';
+}
+elsif ($target =~ m/\.[hc]$/ ) {
+$options{file_type} = 'c';
+}
+elsif ($target =~ m/\.pmc$/ ) {
+$options{file_type} = 'pmc';
+}
+} elsif ( $options{file_type} eq 'none' ) {
+delete $options{file_type};
 }
 
-if ( $options{makefile} ) {
-exists $options{comment_type}  or $options{comment_type}  = '#';
-exists $options{replace_slashes}   or $options{replace_slashes}   = 1;
-exists $options{conditioned_lines} or $options{conditioned_lines} = 1;
+if ( !exists $options{file_type}  $target =~ m/makefile$/i ) {
+$options{file_type} = 'makefile';
 }
 
+if ( $options{file_type} ) {
+unless ( exists $file_types_info{$options{file_type}} ) {
+die Unknown file_type '$options{file_type}';
+}
+unless ( exists $options{comment_type} ) {
+$options{comment_type} = $file_types_info{$options{file_type}}{comment_type};
+}
+if ( $options{file_type} eq 'makefile' ) {
+exists $options{replace_slashes}   or $options{replace_slashes}   = 1;
+exists $options{conditioned_lines} or $options{conditioned_lines} = 1;
+}
+}
+
 if ( $options{comment_type} ) {
-my @comment = ( 'ex: set ro ft=c:',
+my @comment = ( 'ex: set ro',
 'DO NOT EDIT THIS FILE',
 'Generated by ' . __PACKAGE__ .  from $source );
 
+if ($options{file_type}) {
+$comment[0] .= ' ft=' . 

[perl #57884] [PATCH] file_type option for Parrot::Configure::Compiler-genfile

2008-08-13 Thread James Keenan via RT
I think this patch is worth considering and I encourage comments from
those Parrot developers who are knowledgeable about these file coda
issues, how they appear in vi and emacs, etc.

Since this patch is likely to affect a large number of files, I think we
should defer full consideration until after next week's release.  But
comments are welcome in the meantime.

Thank you very much.

kid51


Re: [perl #57884] [PATCH] file_type option for Parrot::Configure::Compiler-genfile

2008-08-13 Thread chromatic
On Wednesday 13 August 2008 17:30:39 James Keenan via RT wrote:

 I think this patch is worth considering and I encourage comments from
 those Parrot developers who are knowledgeable about these file coda
 issues, how they appear in vi and emacs, etc.

 Since this patch is likely to affect a large number of files, I think we
 should defer full consideration until after next week's release.  But
 comments are welcome in the meantime.

+1 to the patch and +1 to waiting a week, though if it went in tonight and we 
had several days of successful Smolders, I wouldn't object either.

-- c