Re: [PATCH v2 1/8] Add new git-cc-cmd helper to contrib

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 19, 2013, 17:08 UTC
Message-ID: <7vfvym30t8.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1366348458-7706-2-git-send-email-felipe.contreras@gmail.com>

Felipe Contreras <felipe.contreras@gmail.com> writes:

> The code finds the changes of a commit, runs 'git blame' for each chunk
> to see which other commits are relevant, and then reports the author and
> signers.

In general, I am not all that interested in adding anything new to contrib/ as git.git has matured enough, but even if this will stay outside my tree, there are a few interesting things to note to help its eventual users.

> +    roles = roles.map do |person, role|
> +      address = "%s <%s>" % person
> +      [person, role]
> +    end

Is address being used elsewhere, or is this a remnant from an earlier debugging or something?

Show 12 quoted lines

> +    [id, roles]
> +  end
> +
> +end
> ...
> +    File.open(file) do |f|
> +      f.each do |line|
> +        case line
> +        when /^From (\h+) (.+)$/
> +          from = $1
> +        when /^---\s+(\S+)/
> +          source = $1 != '/dev/null' ? $1[2..-1] : nil

This may need to be tightened if you want to use this on a real-world project (git.git itself does not count ;-); you may see something like:

    diff --git "a/a\"b" "b/a\"b"

(I did an insane pathname 'a"b' to get the above example, but a more realistic is a character outside ASCII).

> +        when /^@@\s-(\d+),(\d+)/
> +          get_blame(source, $1, $2, from)

This may want to be a bit more careful for a hunk that adds to an empty file, which will give you something like

    @@ -0,0 +1 @@
    @@ -0,0 +1,200 @@

Nobody sane would use -U0 when doing a format-patch, but if this wants to accomodate such a patch as well, it needs to ignore a hunk that only adds new lines.