Re: peer review for first CPAN module? (JavaScript minification)
Joshua ben Jore schreef: Adriano Ferreira: Peter Michaux: I'm writing a new version of JavaScript::Minification on CPAN. This is my first CPAN module and first Perl project! If someone is willing to take a look to see if I've done something terribly wrong packaging the code I would greatly appreciate any feedback. Currently the new module is in a subversion repository. If you have subversion you should be able to check it out and test it with svn co http://dev.michaux.ca/svn/random/JavaScript-Minifier cd JavaScript-Minifier perl MakeFile.PL make test Some comments on packaging: * it may be a good idea to add a LICENSE parameter to Makefile.PL (supported by ExtUtils::MakeMaker = 6.31) * you might like to add POD and POD coverage tests (for CPANTS' sake) Some comments on code: * it does not look like Perl code, but C code translated literally to Perl. That's because this is a translation of the C program, jsmin. http://javascript.crockford.com/jsmin.html The code contains the strange '\000'. I see basically 3 steps: 1. respect strings 2. remove comments 3. compress everything else See also Javascript::Squish. http://search.cpan.org/perldoc?JavaScript::Squish -- Affijn, Ruud Gewoon is een tijger.
Re: peer review for first CPAN module? (JavaScript minification)
On 5/25/07, Dr.Ruud [EMAIL PROTECTED] wrote: Joshua ben Jore schreef: Adriano Ferreira: Peter Michaux: I'm writing a new version of JavaScript::Minification on CPAN. This is my first CPAN module and first Perl project! If someone is willing to take a look to see if I've done something terribly wrong packaging the code I would greatly appreciate any feedback. Currently the new module is in a subversion repository. If you have subversion you should be able to check it out and test it with svn co http://dev.michaux.ca/svn/random/JavaScript-Minifier cd JavaScript-Minifier perl MakeFile.PL make test Some comments on packaging: * it may be a good idea to add a LICENSE parameter to Makefile.PL (supported by ExtUtils::MakeMaker = 6.31) * you might like to add POD and POD coverage tests (for CPANTS' sake) Some comments on code: * it does not look like Perl code, but C code translated literally to Perl. That's because this is a translation of the C program, jsmin. http://javascript.crockford.com/jsmin.html The code contains the strange '\000'. I see basically 3 steps: 1. respect strings 2. remove comments 3. compress everything else You may want want to convert from UTF16 to perl native strings, then convert back. JavaScript is defined as being stored in UTF16 but that's kind of unusual elsewhere. Isn't it? Josh
Re: peer review for first CPAN module? (JavaScript minification)
Joshua ben Jore schreef: Dr.Ruud: I see basically 3 steps: 1. respect strings 2. remove comments 3. compress everything else You may want want to convert from UTF16 to perl native strings, then convert back. JavaScript is defined as being stored in UTF16 but that's kind of unusual elsewhere. Isn't it? AFAIK, the UTF-16 of JavaScript is internal. So you can just put abc in your JavaScript code, there is no need to make that \0a\0b\0c. :) -- Affijn, Ruud Gewoon is een tijger.
Re: peer review for first CPAN module? (JavaScript minification)
On 5/25/07, Joshua ben Jore [EMAIL PROTECTED] wrote: You may want want to convert from UTF16 to perl native strings, then convert back. JavaScript is defined as being stored in UTF16 but that's kind of unusual elsewhere. Isn't it? As Dr. Ruud said; also, ECMAscript engine authors (at least David Leonard) seem to regard normalizing everything to UTF8 as equally bizarre. The Xerxes XML parser, for instance, uses 16 internally.
Re: peer review for first CPAN module? (JavaScript minification)
On 5/24/07, Peter Michaux [EMAIL PROTECTED] wrote:
Hi,
I'm writing a new version of JavaScript::Minification on CPAN. This is
my first CPAN module and first Perl project! If someone is willing to
take a look to see if I've done something terribly wrong packaging the
code I would greatly appreciate any feedback.
Currently the new module is in a subversion repository. If you have
subversion you should be able to check it out and test it with
svn co http://dev.michaux.ca/svn/random/JavaScript-Minifier
cd JavaScript-Minifier
perl MakeFile.PL
make test
Some comments on packaging:
* it may be a good idea to add a LICENSE parameter to Makefile.PL
(supported by ExtUtils::MakeMaker = 6.31)
* you might like to add POD and POD coverage tests (for CPANTS' sake)
Some comments on code:
* it does not look like Perl code, but C code translated literally to Perl.
If you review the code using a Perlish approach, you may be surprised
about how short the result may be. For example,
sub isInfix {
my $x = shift;
return ($x eq ',' || $x eq '=' || $x eq ';' ||
$x eq '?' || $x eq ':' || $x eq '' ||
$x eq '%' || $x eq '*' || $x eq '|' ||
$x eq '' || $x eq '' || $x eq \n);
}
may be replaced by code using regular expressions
sub isInfix {
my $x = shift;
$x =~ /^[,=;?:%*|\n]$/
}
But this in turn may be further improved by not working in a
character-by-character base as the current code does. For instance,
you may drop the anchors in the regexp above and even the subroutine
itself and replace it with the regexp itself (avoiding sub calls):
my $is_infix_re = qr/[,=;?:%*|\n]/;
# and where you would say isInfix($x), the call would be replaced by
# $x =~ / $is_infix_re /xms or something like that
Best,
Adriano.
Thank you,
Peter
--
http://peter.michaux.ca
http://forkjavascript.org
Re: peer review for first CPAN module? (JavaScript minification)
Peter Michaux wrote:
Hi,
I'm writing a new version of JavaScript::Minification on CPAN. This is
my first CPAN module and first Perl project! If someone is willing to
take a look to see if I've done something terribly wrong packaging the
code I would greatly appreciate any feedback.
Currently the new module is in a subversion repository. If you have
subversion you should be able to check it out and test it with
svn co http://dev.michaux.ca/svn/random/JavaScript-Minifier
cd JavaScript-Minifier
perl MakeFile.PL
make test
1. Makefile.PL: Do you really need to use Perl version 5.008006 to use
this module? Why won't just 5.8, or 5.6 do?
2. README: Replace boilerplate content.
3. POD: section on EXPORT should indicate which subs/variables are
exportable on demand.
4. See attached for test coverage. Quite good for a first effort, and
better than most CPAN distros. However, the excessive, C-ish use of ||
conditions mentioned by Adriano lowers your condition coverage and means
your code is actually less well tested than it may first appear.
5. This is perfectly acceptable for a 0.01. Go ahead and upload it to
CPAN so that others have easier access to it and can send you patches.
Thank you very much.
Jim Keenan
Reading database from /Users/jimk/work/jsm/cover_db
-- -- -- -- -- -- --
File stmt bran condsubpod time total
-- -- -- -- -- -- --
...ib/JavaScript/Minifier.pm 97.3 88.8 79.0 100.00.0 100.0 83.9
Total 97.3 88.8 79.0 100.00.0 100.0 83.9
-- -- -- -- -- -- --
Run: t/JavaScript-Minifier.t
Perl version: 5.8.8
OS: darwin
Start:Thu May 24 23:02:16 2007
Finish: Thu May 24 23:02:16 2007
blib/lib/JavaScript/Minifier.pm
line err stmt bran condsubpod time code
1 package
JavaScript::Minifier;
2
3 1119 use strict;
1 4
1 24
4 1122 use warnings;
1 3
1 20
5
6 require Exporter;
7 our @ISA = qw(Exporter);
8 our @EXPORT_OK =
qw(minify);
9
10our $VERSION = '1.0';
11
12#
-
13
14sub isAlphanum {
15***327 327 0974 my $x = shift;
16 #return true if the
character is allowed in identifier.
17 327 10017422 return (($x ge 'a'
$x le 'z') || ($x ge '0' $x le '9') ||
100
100
100
100
*** 66
*** 66
*** 66
*** 66
18 ($x ge 'A'
$x le 'Z') || $x eq '_' || $x eq '$' ||
19 $x eq '\\' ||
ord($x) 126);
20}
21
22sub isSpace {
23*** 2067 2067 0 13590 my $x = shift;
24*** 20676631917 return ($x eq ' ' || $x
eq \t);
25}
26
27sub isEndspace {
28*** 2069 2069 0 5538 my $x = shift;
29*** 20696651764 return ($x eq \n ||
$x eq \r || $x eq \f);
*** 66
30
