Re: My custom cccmd

[Felipe Contreras <felipe.contreras@gmail.com>]

On Thu, Oct 15, 2009 at 11:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> 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