Bug #104512 for JavaScript-Beautifier: fix for broken -o option in js

Tue May 19 02:33:52 2015 d.shrimpton [...] its.uq.edu.au - Ticket created

Subject:	fix for broken -o option in js_beautify.pl
Date:	Tue, 19 May 2015 16:33:40 +1000 (AEST)
To:	bug-JavaScript-Beautifier [...] rt.cpan.org
From:	David Shrimpton <d.shrimpton [...] its.uq.edu.au>

Hi, Fix for small bug in JavaScript::Beautifier 0.17 bin/js_beautify.pl -o outputfile option doesn't work. The input file is always written over. A fix is below: diff -c js_beautify.pl js_beautify.pl.orig *** js_beautify.pl 2015-05-19 16:18:50.869954709 +1000 --- js_beautify.pl.orig 2009-12-30 12:47:20.000000000 +1000 *************** *** 44,50 **** if ( $params{output} or $params{o} ) { my $to_file = $params{output} || $file; ! open(my $fh, '>', $to_file); print $fh $pretty_js; close($fh); } else { --- 44,50 ---- if ( $params{output} or $params{o} ) { my $to_file = $params{output} || $file; ! open(my $fh, '>', $file); print $fh $pretty_js; close($fh); } else { *************** -- David Shrimpton

Tue May 19 03:09:27 2015 fayland [...] cpan.org - Correspondence added

Actually -o is overwrite. and --output is to create new file =item B<-o>, B<--output> By default, we will print beautified javascript to STDOUT if B<-o>, it will override the C<FILE> if B<-output=newfile.js>, it will write into C<newfile.js> Thanks On Tue May 19 02:33:52 2015, d.shrimpton@its.uq.edu.au wrote: Show quoted text

> Hi, > > Fix for small bug in JavaScript::Beautifier 0.17 > > bin/js_beautify.pl -o outputfile option doesn't work. > The input file is always written over. > > A fix is below: > > > diff -c js_beautify.pl js_beautify.pl.orig > > *** js_beautify.pl 2015-05-19 16:18:50.869954709 +1000 > --- js_beautify.pl.orig 2009-12-30 12:47:20.000000000 +1000 > *************** > *** 44,50 **** > > if ( $params{output} or $params{o} ) { > my $to_file = $params{output} || $file; > ! open(my $fh, '>', $to_file); > print $fh $pretty_js; > close($fh); > } else { > --- 44,50 ---- > > if ( $params{output} or $params{o} ) { > my $to_file = $params{output} || $file; > ! open(my $fh, '>', $file); > print $fh $pretty_js; > close($fh); > } else { > *************** > > >

Tue May 19 03:09:28 2015 The RT System itself - Status changed from 'new' to 'open'

Tue May 19 03:09:41 2015 fayland [...] cpan.org - Status changed from 'open' to 'rejected'

Tue May 19 04:05:19 2015 d.shrimpton [...] its.uq.edu.au - Correspondence added

Subject:	Re: [rt.cpan.org #104512] fix for broken -o option in js_beautify.pl
Date:	Tue, 19 May 2015 18:04:56 +1000 (AEST)
To:	Bugs in JavaScript-Beautifier via RT <bug-JavaScript-Beautifier [...] rt.cpan.org>
From:	David Shrimpton <d.shrimpton [...] its.uq.edu.au>

Thanks, I correct my earlier advice . The -output filename does not work, it overwrites the original file . The -o for overwrite works, it overwrites the original file. The patch fixes the problem with -output. David

Tue May 19 04:09:30 2015 fayland [...] cpan.org - Correspondence added

➜ tmp echo 'var a=1+1;' > test.js ➜ tmp js_beautify.pl test.js var a = 1 + 1;% ➜ tmp js_beautify.pl --output test_new.js test.js ➜ tmp cat test.js var a=1+1; ➜ tmp cat test_new.js var a = 1 + 1;% ➜ tmp js_beautify.pl -o test.js ➜ tmp cat test.js var a = 1 + 1;% maybe the doc is not saying it correctly? but the way is right. --output requires an arg. Thanks On Tue May 19 04:05:19 2015, d.shrimpton@its.uq.edu.au wrote: Show quoted text

> Thanks, > > I correct my earlier advice . The -output filename does not work, it > overwrites > the original file . The -o for overwrite works, it overwrites the > original file. > > The patch fixes the problem with -output. > > > David

Tue May 19 04:31:32 2015 d.shrimpton [...] its.uq.edu.au - Correspondence added

Subject:	Re: [rt.cpan.org #104512] fix for broken -o option in js_beautify.pl
Date:	Tue, 19 May 2015 18:31:06 +1000 (AEST)
To:	Fayland Lin via RT <bug-JavaScript-Beautifier [...] rt.cpan.org>
From:	David Shrimpton <d.shrimpton [...] its.uq.edu.au>

Thanks, it doesn't work when I try , so we must have different js_beautify.pl. cat test.js echo 'var a=1+1;' js_beautify.pl --output test_new.js test.js ls test.js cat test.js echo 'var a=1+1;' The file test.js has had a newline removed from the end but has not been beautified as in your example. There is no test_new.js. The original code in the patch always writes to the original file and does not use the value of the --output paramater. On Tue, 19 May 2015, Fayland Lin via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=104512 > > > ➜ tmp echo 'var a=1+1;' > test.js > ➜ tmp js_beautify.pl test.js > var a = 1 + 1;% ➜ tmp js_beautify.pl --output test_new.js test.js > ➜ tmp cat test.js > var a=1+1; > ➜ tmp cat test_new.js > var a = 1 + 1;% ➜ tmp js_beautify.pl -o test.js > ➜ tmp cat test.js > var a = 1 + 1;% > > maybe the doc is not saying it correctly? but the way is right. --output requires an arg. > > Thanks > > On Tue May 19 04:05:19 2015, d.shrimpton@its.uq.edu.au wrote:

> > Thanks, > > > > I correct my earlier advice . The -output filename does not work, it > > overwrites > > the original file . The -o for overwrite works, it overwrites the > > original file. > > > > The patch fixes the problem with -output. > > > > > > David

> > > >

Tue May 19 04:38:36 2015 fayland [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #104512] fix for broken -o option in js_beautify.pl
Date:	Tue, 19 May 2015 16:36:46 +0800
To:	bug-JavaScript-Beautifier [...] rt.cpan.org
From:	Fayland Lam <fayland [...] gmail.com>

could you try Dumper %params then see? maybe an upgrade on Getopt::Long would fix the issue? Thanks On Tue, May 19, 2015 at 4:31 PM, David Shrimpton via RT <bug-JavaScript-Beautifier@rt.cpan.org> wrote: Show quoted text

> Queue: JavaScript-Beautifier > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=104512 > > > Thanks, it doesn't work when I try , so we must have different js_beautify.pl. > > cat test.js > echo 'var a=1+1;' > js_beautify.pl --output test_new.js test.js > ls > test.js > cat test.js > echo 'var a=1+1;' > > The file test.js has had a newline removed from the end but has not been beautified > as in your example. There is no test_new.js. > > The original code in the patch always writes to the original file and does > not use the value of the --output paramater. > > > > > On Tue, 19 May 2015, Fayland Lin via RT wrote: >

>> <URL: https://rt.cpan.org/Ticket/Display.html?id=104512 > >> >> ➜ tmp echo 'var a=1+1;' > test.js >> ➜ tmp js_beautify.pl test.js >> var a = 1 + 1;% ➜ tmp js_beautify.pl --output test_new.js test.js >> ➜ tmp cat test.js >> var a=1+1; >> ➜ tmp cat test_new.js >> var a = 1 + 1;% ➜ tmp js_beautify.pl -o test.js >> ➜ tmp cat test.js >> var a = 1 + 1;% >> >> maybe the doc is not saying it correctly? but the way is right. --output requires an arg. >> >> Thanks >> >> On Tue May 19 04:05:19 2015, d.shrimpton@its.uq.edu.au wrote:

>> > Thanks, >> > >> > I correct my earlier advice . The -output filename does not work, it >> > overwrites >> > the original file . The -o for overwrite works, it overwrites the >> > original file. >> > >> > The patch fixes the problem with -output. >> > >> > >> > David

>> >> >> >>

-- Fayland Lam // http://www.fayland.org/

Tue May 19 05:07:58 2015 d.shrimpton [...] its.uq.edu.au - Correspondence added

Subject:	Re: [rt.cpan.org #104512] fix for broken -o option in js_beautify.pl
Date:	Tue, 19 May 2015 19:07:47 +1000 (AEST)
To:	"fayland [...] gmail.com via RT" <bug-JavaScript-Beautifier [...] rt.cpan.org>
From:	David Shrimpton <d.shrimpton [...] its.uq.edu.au>

Show quoted text

> could you try Dumper %params then see? > > maybe an upgrade on Getopt::Long would fix the issue? >

ls -l test.js 18 May 19 18:57 test.js cat test.js | od -c 0000000 e c h o ' v a r a = 1 + 1 ; 0000020 ' \n 0000022 js_beautify.pl --output test_new.js test.js ls -l test.js May 19 18:59 test.js cat test.js | od -c 0000000 e c h o ' v a r a = 1 + 1 ; 0000020 ' 0000021 ls -l test_new.js ls: cannot access test_new.js: No such file or directory I put some print debugging in js_beautify.pl: if ( $params{output} or $params{o} ) { my $to_file = $params{output} || $file; print "file:$file\n"; foreach my $k (keys %params) { print "key:$k value:$params{$k}\n"; } open(my $fh, '>', $file); print $fh $pretty_js; close($fh); } else { print $pretty_js; } The output is : file:test.js key:output value:test_new.js The problem is not the argument processing. As per the patch the code that writes the output is broken, it is writing to $file and not $to_file and therefore $params{output} is ignored and $file written to always.

Tue May 19 05:13:40 2015 fayland [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #104512] fix for broken -o option in js_beautify.pl
Date:	Tue, 19 May 2015 17:12:49 +0800
To:	bug-JavaScript-Beautifier [...] rt.cpan.org
From:	Fayland Lam <fayland [...] gmail.com>

Sorry, you're right. that was fixed in git: https://github.com/fayland/perl-javascript-beautifier/blob/master/bin/js_beautify.pl and I was using the version from there. now a new version is shipped. Thanks a lot! On Tue, May 19, 2015 at 5:07 PM, David Shrimpton via RT <bug-JavaScript-Beautifier@rt.cpan.org> wrote: Show quoted text

> Queue: JavaScript-Beautifier > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=104512 > >

>> could you try Dumper %params then see? >> >> maybe an upgrade on Getopt::Long would fix the issue? >>

> > ls -l test.js > 18 May 19 18:57 test.js > > cat test.js | od -c > 0000000 e c h o ' v a r a = 1 + 1 ; > 0000020 ' \n > 0000022 > > js_beautify.pl --output test_new.js test.js > > ls -l test.js > May 19 18:59 test.js > > cat test.js | od -c > 0000000 e c h o ' v a r a = 1 + 1 ; > 0000020 ' > 0000021 > > ls -l test_new.js > ls: cannot access test_new.js: No such file or directory > > > I put some print debugging in js_beautify.pl: > > > > if ( $params{output} or $params{o} ) { > my $to_file = $params{output} || $file; > > print "file:$file\n"; > foreach my $k (keys %params) { > print "key:$k value:$params{$k}\n"; > } > open(my $fh, '>', $file); > print $fh $pretty_js; > close($fh); > } else { > print $pretty_js; > } > > The output is : > > file:test.js > key:output value:test_new.js > > > The problem is not the argument processing. As per the patch > the code that writes the output is broken, it is writing to > $file and not $to_file and therefore $params{output} is ignored > and $file written to always. >

-- Fayland Lam // http://www.fayland.org/

Bug #104512 for JavaScript-Beautifier: fix for broken -o option in js_beautify.pl