tools/docs/filename_and_chapter.pl: Can we make this work with our coding standards?

[jkeenan]

We appear to have a disagreement over whether a particular file in our distribution can both work and conform to our coding standards for code written in Perl 5.

The file in question is tools/docs/filename_and_chapter.pl, which was first contributed by gerd in r45489 on April 09. The purpose of this program, according to the initial commit, is "to generate a PDF file as Parrot book from POD files."

mikehh fixed some coding standards failures in r45505 on April 10. Running make codetest two days ago, I noticed codingstd failures with this file, which I corrected in r46223 (trailing whitespace) and r46224 (global filehandles; two-argument form of open). I noted in my commit message that the file still failed to pass perlcritic.t due to use of subroutine prototypes. In r46257, chromatic removed the unnecessary prototypes and did some other cleanup.

But today in r46280, gerd reverted in one instance to a global filehandle and to the two-argument form of open, arguing in his commit message that the program did not work otherwise.

svn diff -r 46257 tools/docs/filename_and_chapter.pl
# ...
@@ -68,17 +68,20 @@
 
     open( my $IN_FH, '<', $item_list_ref->[$_[0]][0] ) or
         die( "$0: can't open $item_list_ref->[$_[0]][0] for reading ($!)\n" );
-    open( my $OUT_FH, '>', "${MOD_BUILD_PATH}$item_list_ref->[$_[0]][0]" );
+    open( OUT_FH, ">${MOD_BUILD_PATH}$item_list_ref->[$_[0]][0]" );  # <-- line 71
 
     # do the same as: sed -e '4,6c\=head0 $item_list_ref->[$i][1]'
     while( <$IN_FH> ) {
         if ( $icnt = (4..6) ) {
             if ( $icnt =~ /E0$/ ) {
-                print( $OUT_FH "=head0 $item_list_ref->[$_[0]][1]\n");
+                print( OUT_FH "=head0 $item_list_ref->[$_[0]][1]\n");
             }
         }
-        else { print( $OUT_FH ); }
+        else { print( OUT_FH ); }
     }
+
+    close( $IN_FH );
+    close( OUT_FH );
 }

I suspect that we could move to the preferred, 3-argument form of open at line 71 without a problem. But I don't see why we need to use a global filehandle to print output rather than the preferred lexically scoped filehandle.

Since we've been back-and-forth on this via commit messages, I think we would be better off if we converted this discussion to a TT.

Gerd,

Could you elaborate a bit more on the purpose of this program and why it won't work with lexical filehandles?

Thank you very much.

kid51

[particle]

it seems to me that

open( my $OUT_FH, '>', "${MOD_BUILD_PATH}$item_list_ref->[$_[0]][0]" );

has my incorrectly scoped, so a "Useless use of constant in void context" warning is emitted. i see no other reason for that code to fail. try

open( my ($OUT_FH), '>', "${MOD_BUILD_PATH}$item_list_ref->[$_[0]][0]" );

or better yet, don't declare a variable inside an op parameter list, do it first:

my $OUT_FH;
open( $OUT_FH, '>', "${MOD_BUILD_PATH}$item_list_ref->[$_[0]][0]" );

[gerd]

The script writes modified POD files to build/modified_pod before it converts them to tex. Chromatic already fixed it correct in r46292 now. ($_ was necessary.) So I close this ticket.

Thanks

Gerd