Re: My custom cccmd

On Thu, Oct 15, 2009 at 11:09 PM, Junio C Hamano [off-list ref] wrote:

Felipe Contreras [off-list ref] writes:

quoted

Hi,

I love the new option to run a cccmd and how good it works on the
linux kernel, but I couldn't find a generic script. So I decided to
write my own.

It's very simple, it just looks into the authors of the commits that
modified the lines being overridden (git blame). It's not checking for
s-o-b, or anything fancy.

<snip/>

Comments.

 #0. Gaahhh, my eyes, my eyes!!  Can't you do this ugly run of infinite
    number of "end"s?

Hehe, sure. Will do.

 #1. You are not making sure that you start blaming from the commit the
    patch is based on, so your -La,b line numbers can be off.  If you can
    assume that you are always reading format-patch output, you can learn
    which commit to start from by reading the first "magic" line.

The 'From' magic line points to the actual commit the patch was
generated from, so it would actually be @from^.

This of course would only work if the patches have the corresponding
commits in the current tree (which is the case most of the time).

And makes sense only for the first patch, the rest of the patches
would use a wrong commit as a base. See below.

 #2. If you have two patch series that updates one file twice, some
    changes in your second patch could even be an update to the changes
    you introduced in your first patch.  After you fix issue #1, you
    would probably want to fix this by excluding the commits you have
    already sent the blames for.

How exactly? By looking at the commit from 'git blame' and discarding
it? That would be a bit tricky since each instance of 'cccmd' is not
aware of the previous ones.

 #3. Does the number of commits you keep per author have any significance?
    I know it doesn't in the implementation you posted, but should it,
    and if so how?

Not currently. Once I add support for s-o-b it might be useful.
Currently I left it in order to order the CC's by the count, but it
turned out to be a bit messier than I thought, and the advantage is
almost nothing.

I'll clean it up.

Taking in consideration the previous comments, here is v2:

#!/usr/bin/env ruby

@authors = {}

def parse_blame(line)
  key, value = line.split(" ", 2)
  case key
  when "author"
    @name = value
  when "author-mail"
    @mail = value
    author = "\"#{@name}\" #{@mail}"
    @authors[author] = true
  end
end

ARGV.each do |filename|
  patch_file = File.open(filename)
  patch_file.each_line do |patch_line|
    case patch_line
    when /^---\s+(\S+)/
      @source = $1[2..-1]
    when /^@@\s-(\d+),(\d+)/
      blame = `git blame -p -L #{$1},+#{$2} #{@source} | grep author`
      blame.each_line { |l| parse_blame(l.chomp) }
    end
  end
end

@authors.each_key do |a|
  puts a
end

-- 
Felipe Contreras