rename_rev.pl script for reviewing renames

rename_rev.pl script for reviewing renames

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 535 bytes --]

There are a lot of refactoring patches where people change camel case
names to kernel style names etc.  I've written a script to make it
easier to review them.  It's attached.

For example the linked patch is about 7900 lines long.  It renames
A_BOOL to bool, TRUE to true and FALSE to false.
http://driverdev.linuxdriverproject.org/pipermail/devel/2011-February/011810.html

cat email.txt | ./rename_rev.pl A_BOOL bool TRUE true FALSE false | less

The resulting diff is 78 lines long (99% reduced).  Woohoo!

regards,
dan carpenter


[-- Attachment #2: rename_rev.pl --]
[-- Type: text/x-perl, Size: 1109 bytes --]

#!/usr/bin/perl

use File::Temp qw/ :mktemp  /;

sub usage() {
    print "cat diff | transform.pl old new old new old new...\n";
    die;
}

my @subs;

sub filter($) {
    my $line = shift();

    foreach my $sub (@subs) {
	$line =~ s/$sub->[0]/$sub->[1]/g;
    }

    # white space at the end of lines
    $line =~ s/ *$//g;
    $line =~ s/\t*$//g;

    # remove the first char
    $line =~ s/^[ +-]//;

    # tabs to spaces
    $line =~ s/\ {8}/\t/g;

    return $line;
}

($oldfh, $oldfile) = mkstemp("/tmp/oldXXXXX");
($newfh, $newfile) = mkstemp("/tmp/newXXXXX");

while (my $param1 = shift()) {
    my $param2 = shift;

    if ($param2 =~ /^$/) {
	usage();
    }
    push @subs, [$param1, $param2];
}

while (<>) {
    my $line = $_;

    if ($line =~ /^---/) {
	next;
    }
    if ($line =~ /^\+\+\+/) {
	next;
    }

    my $output = filter($line);
    if ($line =~ /^-/) {
	print $oldfh $output;
	next;
    }
    if ($line =~ /^\+/) {
	print $newfh $output;
	next;
    }
    print $oldfh $output;
    print $newfh $output;
}

system("diff -u $oldfile $newfile");

unlink($oldfile);
unlink($newfile);

Re: rename_rev.pl script for reviewing renames

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1273 bytes --]

On Thu, Feb 03, 2011 at 01:08:28PM +0300, Dan Carpenter wrote:
> There are a lot of refactoring patches where people change camel case
> names to kernel style names etc.  I've written a script to make it
> easier to review them.  It's attached.

Cool, thanks for sharing. I guess my comments won't matter much, here
they are anyway :)

> sub usage() {
>     print "cat diff | transform.pl old new old new old new...\n";

Filename of the tool does not match.

>     # white space at the end of lines
>     $line =~ s/ *$//g;
>     $line =~ s/\t*$//g;

Character class here as well? Will also get mixtures of the two.

> while (<>) {
>     my $line = $_;

You could work here with plain $_, but I assume you don't do on purpose.

> 
>     if ($line =~ /^---/) {
> 	next;
>     }
>     if ($line =~ /^\+\+\+/) {
> 	next;
>     }

Use an alternation in the regexp?

>     my $output = filter($line);
>     if ($line =~ /^-/) {
> 	print $oldfh $output;
> 	next;
>     }
>     if ($line =~ /^\+/) {
> 	print $newfh $output;
> 	next;
>     }

Ditto.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: rename_rev.pl script for reviewing renames

[Dan Carpenter]

On Thu, Feb 03, 2011 at 11:22:19AM +0100, Wolfram Sang wrote:
> >     # white space at the end of lines
> >     $line =~ s/ *$//g;
> >     $line =~ s/\t*$//g;
> 
> Character class here as well? Will also get mixtures of the two.
> 

Actually it's better to let diff to take care of white space probably.
I changed the diff at the end to "diff -uw" and it seems to work
beautifully.

regards,
dan carpenter

Re: rename_rev.pl script for reviewing renames

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 485 bytes --]

On Thu, Feb 03, 2011 at 11:22:19AM +0100, Wolfram Sang wrote:
> On Thu, Feb 03, 2011 at 01:08:28PM +0300, Dan Carpenter wrote:
> > There are a lot of refactoring patches where people change camel case
> > names to kernel style names etc.  I've written a script to make it
> > easier to review them.  It's attached.
> 
> Cool, thanks for sharing. I guess my comments won't matter much, here
> they are anyway :)
> 

Thanks for the review.  I made the changes.  

regards,
dan carpenter

[-- Attachment #2: rename_rev.pl --]
[-- Type: text/x-perl, Size: 942 bytes --]

#!/usr/bin/perl

use File::Temp qw/ :mktemp  /;

sub usage() {
    print "usage: cat diff | $0 old new old new old new...\n";
    exit(1);
}

my @subs;

sub filter($) {
    my $line = shift();

    foreach my $sub (@subs) {
	$line =~ s/$sub->[0]/$sub->[1]/g;
    }

    # remove the first char
    $line =~ s/^[ +-]//;

    return $line;
}

($oldfh, $oldfile) = mkstemp("/tmp/oldXXXXX");
($newfh, $newfile) = mkstemp("/tmp/newXXXXX");

while (my $param1 = shift()) {
    my $param2 = shift;

    if ($param2 =~ /^$/) {
	usage();
    }
    push @subs, [$param1, $param2];
}

while (<>) {
    my $line = $_;

    if ($line =~ /^(---|\+\+\+)/) {
	next;
    }

    my $output = filter($line);
    if ($line =~ /^-/) {
	print $oldfh $output;
	next;
    }
    if ($line =~ /^\+/) {
	print $newfh $output;
	next;
    }
    print $oldfh $output;
    print $newfh $output;
}

system("diff -uw $oldfile $newfile");

unlink($oldfile);
unlink($newfile);

Re: rename_rev.pl script for reviewing renames

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 1020 bytes --]

I've added the some flags:
	-e : gets executed on old lines
	-ea: executed on everything
	-nc: strips one line comments

So say you have a patch that adds parenthesis around a bunch of macros
like this:
http://marc.info/?l=linux-kernel&m=129685142419550&w=2

Then you can just type:
cat /home/dcarpenter/tmp/html23/parens.patch | \
	./rename_rev.pl -e 's/(define \w+) (.*)/$1 ($2)/'

Or if you have a camel case script that changes "ThisVariable" to
"this_variable".  Then the command would be:
git show c659c38 | ./rename_rev.pl -ea '$_ = lc' -ea 's/_//g'
Which changes everything to lower case and strips out all the
underscores.  You might want to combine it with some other flags:
git show c659c38 | ./rename_rev.pl -nc \
	-e 's/TLanPrivateInfo/struct tlan_priv/' \
	-e 's/TLanList/struct tlan_list/' \
	-ea '$_ = lc' -ea 's/_//g'

What I would like is if there was some way to ignore changes which just
introduced new lines, but didn't affect runtime behavior.  I'm not sure
how to do that.

regards,
dan carpenter

[-- Attachment #2: rename_rev.pl --]
[-- Type: text/x-perl, Size: 1580 bytes --]

#!/usr/bin/perl

use strict;
use File::Temp qw/ :mktemp  /;

sub usage() {
    print "usage: cat diff | $0 old new old new old new...\n";
    print "   or: cat diff | $0 -e 's/old/new/g'\n"; 
    print " -e : execute on old lines\n";
    print " -ea: execute on all lines\n";
    print " -nc: strip one line comments\n"; 
    exit(1);
}

my @subs;
my @cmds;
my $strip_comments;

sub filter($) {
    my $_ = shift();

    my $old = 0;
    if ($_ =~ /^-/) {
        $old = 1;
    }

    # remove the first char
    s/^[ +-]//;

    if ($strip_comments) {
        s/\/\*.*?\*\///g;
        s/\/\/.*//;
    }

    foreach my $cmd (@cmds) {
        if ($old || $cmd->[0] =~ /^-ea$/) {
		eval $cmd->[1];
	}
    }

    foreach my $sub (@subs) {
	if ($old) {
		s/$sub->[0]/$sub->[1]/g;
	}
    }

    return $_;
}


my ($oldfh, $oldfile) = mkstemp("/tmp/oldXXXXX");
my ($newfh, $newfile) = mkstemp("/tmp/newXXXXX");

while (my $param1 = shift()) {
    if ($param1 =~ /^-nc$/) {
        $strip_comments = 1;
        next;
    }

    my $param2 = shift;
    if ($param2 =~ /^$/) {
	usage();
    }

    if ($param1 =~ /^-e(a|)$/) {
        push @cmds, [$param1, $param2];
	next;
    }
    push @subs, [$param1, $param2];
}

while (<>) {
    my $line = $_;

    if ($line =~ /^(---|\+\+\+)/) {
	next;
    }

    my $output = filter($line);
    if ($line =~ /^-/) {
	print $oldfh $output;
	next;
    }
    if ($line =~ /^\+/) {
	print $newfh $output;
	next;
    }
    print $oldfh $output;
    print $newfh $output;
}

system("diff -uw $oldfile $newfile");

unlink($oldfile);
unlink($newfile);

Re: rename_rev.pl script for reviewing renames

[Joe Perches]

On Sun, 2011-02-06 at 15:25 +0300, Dan Carpenter wrote:
> Or if you have a camel case script that changes "ThisVariable" to
> "this_variable".  Then the command would be:
> git show c659c38 | ./rename_rev.pl -ea '$_ = lc' -ea 's/_//g'
> Which changes everything to lower case and strips out all the
> underscores.  You might want to combine it with some other flags:
> git show c659c38 | ./rename_rev.pl -nc \
> 	-e 's/TLanPrivateInfo/struct tlan_priv/' \
> 	-e 's/TLanList/struct tlan_list/' \
> 	-ea '$_ = lc' -ea 's/_//g'

Hi Dan.

I think you'll need to add '\b$1\b' to your tests
otherwise your first example will check things like
"TLanPrivateInfoType" as well.

> What I would like is if there was some way to ignore changes which just
> introduced new lines, but didn't affect runtime behavior.  I'm not sure
> how to do that.

Any object change will affect runtime behavior.