{"thread":{"id":"33863","subject":"[PATCH v5 00/15] New git-related helper","startedAt":"2013-05-18T11:46:40Z","lastAt":"2013-05-19T21:14:18Z","messageCount":28,"participants":["Felipe Contreras","Junio C Hamano","Ramkumar Ramachandra","David Aguilar"],"isPatch":true,"patchVersion":5,"patchTotal":15},"messages":[{"id":"217796","messageId":"1368877615-9563-1-git-send-email-felipe.contreras@gmail.com","threadId":"33863","inReplyTo":null,"subject":"[PATCH v5 00/15] New git-related helper","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:46:40Z","receivedAt":"2013-05-18T11:46:40Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nHere goes version 5. I decided to start with a very very minimal working\nversion that is only 124 lines of code, then slowly but steadily introduce all\nthe fancy features. I also tweaked the defaults so they give more meaninful\nresults (IMO).\n\nI also fixed the parsing of diffs so that it tackles corner case Junio\nmentioned.\n\nThis script allows you to get a list of relevant persons to Cc when sending a\npatch series.\n\n % git related v1.8.1.6^^1..v1.8.1.6^^2\n Junio C Hamano (signer: 92%, author: 7%)\n Nguyễn Thái Ngọc Duy (author: 38%)\n Michael Haggerty (author: 15%)\n\nIt finds people that might be interesting in a patch, by going back through the\nhistory for each single hunk modified, and finding people that reviewed,\nacknowledged, signed, or authored the code the patch is modifying.\n\nIt does this by running 'git blame' incrementally on each hunk, and then\nparsing the commit message. After gathering all the relevant people, it groups\nthem to show what exactly was their role when the participated in the\ndevelopment of the relevant commit, and on how many relevant commits they\nparticipated. They are only displayed if they pass a minimum threshold of\nparticipation.\n\nThe code finds the changes in each commit in the list, runs 'git blame'\nto see which other commits are relevant to those lines, and then adds\nthe author and signer to the list.\n\nFinally, it calculates what percentage of the total relevant commits\neach person was involved in, and if it passes the threshold, it goes in.\n\nYou can also choose to show the commits themselves:\n\n % git related --commits v1.8.1.6^^1..v1.8.1.6^^2\n 9db9eec attr: avoid calling find_basename() twice per path\n 94bc671 Add directory pattern matching to attributes\n 82dce99 attr: more matching optimizations from .gitignore\n 593cb88 exclude: split basename matching code into a separate function\n b559263 exclude: split pathname matching code into a separate function\n 4742d13 attr: avoid searching for basename on every match\n f950eb9 rename pathspec_prefix() to common_prefix() and move to dir.[ch]\n 4a085b1 consolidate pathspec_prefix and common_prefix\n d932f4e Rename git_checkattr() to git_check_attr()\n 2d72174 Extract a function collect_all_attrs()\n 8cf2a84 Add string comparison functions that respect the ignore_case variable.\n 2c5b011 dir.c: Fix two minor grammatical errors in comments\n 377d9c4 Makefile: update the default build options for AIX\n\nBut wait, there's more: you can also specify a list of patch files, which means\nthis can be used for 'git send-emails' --cc-cmd option.\n\nI don't know about you, but I've found this script incredibly useful.\n\nFelipe Contreras (15):\n Add new git-related helper to contrib\n contrib: related: add option parsing\n contrib: related: sort by amount of involvement\n contrib: related: print the amount of involvement\n contrib: related: add helper Person classes\n contrib: related: show role count\n contrib: related: add support for more roles\n contrib: related: group persons with same email\n contrib: related: add mailmap support\n contrib: related: allow usage on other directories\n contrib: related: add support for multiple patches\n contrib: related: add option to show commits\n contrib: related: add option to parse from committish\n contrib: related: parse committish like format-patch\n contrib: related: fix parsing of rev-list args\n\n contrib/related/git-related | 312 ++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 312 insertions(+)\n create mode 100755 contrib/related/git-related\n\n-- \n1.8.3.rc2.542.g24820ba\n"},{"id":"217797","messageId":"1368877615-9563-2-git-send-email-felipe.contreras@gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 01/15] Add new git-related helper to contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:46:41Z","receivedAt":"2013-05-18T11:46:41Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"This script find people that might be interested in a patch, by going\nback through the history for each single hunk modified, and finding\npeople that reviewed, acknowledge, signed, or authored the code the\npatch is modifying.\n\nIt does this by running 'git blame' incrementally on each hunk, and then\nparsing the commit message. After gathering all the relevant people, it\ngroups them to show what exactly was their role when the participated in\nthe development of the relevant commit, and on how many relevant commits\nthey participated. They are only displayed if they pass a minimum\nthreshold of participation.\n\nFor example:\n\n % git related 0001-remote-hg-trivial-cleanups.patch\n Felipe Contreras \n Jeff King \n Max Horn \n Junio C Hamano \n\nThus it can be used for 'git send-email' as a cc-cmd.\n\nThere might be some other related functions to this script, not just to\nbe used as a cc-cmd.\n\nComments-by: Ramkumar Ramachandra \nSigned-off-by: Felipe Contreras \n---\n contrib/related/git-related | 124 ++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 124 insertions(+)\n create mode 100755 contrib/related/git-related\n\ndiff --git a/contrib/related/git-related b/contrib/related/git-related\nnew file mode 100755\nindex 0000000..4f31482\n--- /dev/null\n+++ b/contrib/related/git-related\n@@ -0,0 +1,124 @@\n+#!/usr/bin/env ruby\n+\n+# This script finds people that might be interested in a patch\n+# usage: git related \n+\n+$since = '5-years-ago'\n+$min_percent = 10\n+\n+def fmt_person(name, email)\n+ name ? '%s <%s>' % [name, email] : email\n+end\n+\n+class Commit\n+\n+ attr_reader :persons\n+\n+ def initialize(id)\n+ @id = id\n+ @persons = []\n+ end\n+\n+ def parse(data)\n+ msg = nil\n+ data.each_line do |line|\n+ if not msg\n+ case line\n+ when /^author ([^<>]+) <(\\S+)> (.+)$/\n+ @persons << fmt_person($1, $2)\n+ when /^$/\n+ msg = true\n+ end\n+ else\n+ if line =~ /^(Signed-off|Reviewed|Acked)-by: ([^<>]+) <(\\S+?)>$/\n+ @persons << fmt_person($2, $3)\n+ end\n+ end\n+ end\n+ @persons.uniq!\n+ end\n+\n+end\n+\n+class Commits\n+\n+ def initialize\n+ @items = {}\n+ end\n+\n+ def size\n+ @items.size\n+ end\n+\n+ def each(&block)\n+ @items.each(&block)\n+ end\n+\n+ def import\n+ return if @items.empty?\n+ File.popen(%w[git cat-file --batch], 'r+') do |p|\n+ p.write(@items.keys.join(\"\\n\"))\n+ p.close_write\n+ p.each do |l|\n+ if l =~ /^(\\h{40}) commit (\\d+)/\n+ id, len = $1, $2\n+ data = p.read($2.to_i)\n+ @items[id].parse(data)\n+ end\n+ end\n+ end\n+ end\n+\n+ def get_blame(source, start, len, from)\n+ return if len == 0\n+ len ||= 1\n+ File.popen(['git', 'blame', '--incremental', '-C',\n+ '-L', '%u,+%u' % [start, len],\n+ '--since', $since, from + '^',\n+ '--', source]) do |p|\n+ p.each do |line|\n+ if line =~ /^(\\h{40})/\n+ id = $1\n+ @items[id] = Commit.new(id)\n+ end\n+ end\n+ end\n+ end\n+\n+ def from_patch(file)\n+ from = source = nil\n+ File.open(file) do |f|\n+ f.each do |line|\n+ case line\n+ when /^From (\\h+) (.+)$/\n+ from = $1\n+ when /^---\\s+(\\S+)/\n+ source = $1 != '/dev/null' ? $1[2..-1] : nil\n+ when /^@@ -(\\d+)(?:,(\\d+))?/\n+ get_blame(source, $1, $2, from)\n+ end\n+ end\n+ end\n+ end\n+\n+end\n+\n+exit 1 if ARGV.size != 1\n+\n+commits = Commits.new\n+commits.from_patch(ARGV[0])\n+commits.import\n+\n+count_per_person = Hash.new(0)\n+\n+commits.each do |id, commit|\n+ commit.persons.each do |person|\n+ count_per_person[person] += 1\n+ end\n+end\n+\n+count_per_person.each do |person, count|\n+ percent = count.to_f * 100 / commits.size\n+ next if percent < $min_percent\n+ puts person\n+end\n-- \n1.8.3.rc2.542.g24820ba\n"},{"id":"217798","messageId":"1368877615-9563-3-git-send-email-felipe.contreras@gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 02/15] contrib: related: add option parsing","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:46:42Z","receivedAt":"2013-05-18T11:46:42Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras \n---\n contrib/related/git-related | 17 +++++++++++++++++\n 1 file changed, 17 insertions(+)\n\ndiff --git a/contrib/related/git-related b/contrib/related/git-related\nindex 4f31482..6f18cc8 100755\n--- a/contrib/related/git-related\n+++ b/contrib/related/git-related\n@@ -3,6 +3,8 @@\n # This script finds people that might be interested in a patch\n # usage: git related \n \n+require 'optparse'\n+\n $since = '5-years-ago'\n $min_percent = 10\n \n@@ -10,6 +12,21 @@ def fmt_person(name, email)\n name ? '%s <%s>' % [name, email] : email\n end\n \n+begin\n+ OptionParser.new do |opts|\n+ opts.program_name = 'git related'\n+ opts.banner = 'usage: git related [options] '\n+\n+ opts.on('-p', '--min-percent N', Integer, 'Minium percentage of role participation') do |v|\n+ $min_percent = v\n+ end\n+ opts.on('-d', '--since DATE', 'How far back to search for relevant commits') do |v|\n+ $since = v\n+ end\n+ end.parse!\n+rescue OptionParser::InvalidOption\n+end\n+\n class Commit\n \n attr_reader :persons\n-- \n1.8.3.rc2.542.g24820ba\n"},{"id":"217799","messageId":"1368877615-9563-4-git-send-email-felipe.contreras@gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 03/15] contrib: related: sort by amount of involvement","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:46:43Z","receivedAt":"2013-05-18T11:46:43Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras \n---\n contrib/related/git-related | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/related/git-related b/contrib/related/git-related\nindex 6f18cc8..8c26cf8 100755\n--- a/contrib/related/git-related\n+++ b/contrib/related/git-related\n@@ -134,7 +134,12 @@ commits.each do |id, commit|\n end\n end\n \n-count_per_person.each do |person, count|\n+# sort by number of participations\n+count_sort = lambda do |a, b|\n+ b[1] <=> a[1]\n+end\n+\n+count_per_person.sort(&count_sort).each do |person, count|\n percent = count.to_f * 100 / commits.size\n next if percent < $min_percent\n puts person\n-- \n1.8.3.rc2.542.g24820ba\n"},{"id":"217800","messageId":"1368877615-9563-5-git-send-email-felipe.contreras@gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 04/15] contrib: related: print the amount of involvement","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:46:44Z","receivedAt":"2013-05-18T11:46:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"100% means the person was involved in all the commits, in one way or the\nother.\n\nSigned-off-by: Felipe Contreras \n---\n contrib/related/git-related | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/related/git-related b/contrib/related/git-related\nindex 8c26cf8..7be2829 100755\n--- a/contrib/related/git-related\n+++ b/contrib/related/git-related\n@@ -142,5 +142,5 @@ end\n count_per_person.sort(&count_sort).each do |person, count|\n percent = count.to_f * 100 / commits.size\n next if percent < $min_percent\n- puts person\n+ puts '%s (involved: %u%%)' % [person, percent]\n end\n-- \n1.8.3.rc2.542.g24820ba\n"},{"id":"217801","messageId":"1368877615-9563-6-git-send-email-felipe.contreras@gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 05/15] contrib: related: add helper Person classes","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:46:45Z","receivedAt":"2013-05-18T11:46:45Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"No functional changes.\n\nSigned-off-by: Felipe Contreras \n---\n contrib/related/git-related | 80 +++++++++++++++++++++++++++++++--------------\n 1 file changed, 56 insertions(+), 24 deletions(-)\n\ndiff --git a/contrib/related/git-related b/contrib/related/git-related\nindex 7be2829..df13148 100755\n--- a/contrib/related/git-related\n+++ b/contrib/related/git-related\n@@ -8,10 +8,6 @@ require 'optparse'\n $since = '5-years-ago'\n $min_percent = 10\n \n-def fmt_person(name, email)\n- name ? '%s <%s>' % [name, email] : email\n-end\n-\n begin\n OptionParser.new do |opts|\n opts.program_name = 'git related'\n@@ -27,13 +23,59 @@ begin\n rescue OptionParser::InvalidOption\n end\n \n-class Commit\n+class Person\n+\n+ attr_reader :roles\n+\n+ def initialize(name, email)\n+ @name = name\n+ @email = email\n+ @commits = {}\n+ end\n+\n+ def add_role(commit)\n+ @commits[commit] = true\n+ end\n+\n+ def <=>(b)\n+ self.size <=> b.size\n+ end\n+\n+ def size\n+ @commits.size\n+ end\n+\n+ def to_s\n+ @name ? '%s <%s>' % [@name, @email] : @email\n+ end\n+\n+end\n \n- attr_reader :persons\n+class Persons\n+\n+ @@index = {}\n+\n+ include Enumerable\n+\n+ def each(&block)\n+ @@index.values.each(&block)\n+ end\n+\n+ def self.get(name, email)\n+ id = [name, email]\n+ person = @@index[id]\n+ if not person\n+ person = @@index[id] = Person.new(name, email)\n+ end\n+ person\n+ end\n+\n+end\n+\n+class Commit\n \n def initialize(id)\n @id = id\n- @persons = []\n end\n \n def parse(data)\n@@ -42,17 +84,18 @@ class Commit\n if not msg\n case line\n when /^author ([^<>]+) <(\\S+)> (.+)$/\n- @persons << fmt_person($1, $2)\n+ author = Persons.get($1, $2)\n+ author.add_role(@id)\n when /^$/\n msg = true\n end\n else\n if line =~ /^(Signed-off|Reviewed|Acked)-by: ([^<>]+) <(\\S+?)>$/\n- @persons << fmt_person($2, $3)\n+ person = Persons.get($2, $3)\n+ person.add_role(@id)\n end\n end\n end\n- @persons.uniq!\n end\n \n end\n@@ -126,21 +169,10 @@ commits = Commits.new\n commits.from_patch(ARGV[0])\n commits.import\n \n-count_per_person = Hash.new(0)\n-\n-commits.each do |id, commit|\n- commit.persons.each do |person|\n- count_per_person[person] += 1\n- end\n-end\n-\n-# sort by number of participations\n-count_sort = lambda do |a, b|\n- b[1] <=> a[1]\n-end\n+persons = Persons.new\n \n-count_per_person.sort(&count_sort).each do |person, count|\n- percent = count.to_f * 100 / commits.size\n+persons.sort.reverse.each do |person|\n+ percent = person.size.to_f * 100 / commits.size\n next if percent < $min_percent\n puts '%s (involved: %u%%)' % [person, percent]\n end\n-- \n1.8.3.rc2.542.g24820ba\n"},{"id":"217802","messageId":"1368877615-9563-7-git-send-email-felipe.contreras@gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 06/15] contrib: related: show role count","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:46:46Z","receivedAt":"2013-05-18T11:46:46Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Instead of showing the total involvement, show it per role: author, or\nsigner.\n\nSigned-off-by: Felipe Contreras \n---\n contrib/related/git-related | 18 +++++++++++++-----\n 1 file changed, 13 insertions(+), 5 deletions(-)\n\ndiff --git a/contrib/related/git-related b/contrib/related/git-related\nindex df13148..cd1ef59 100755\n--- a/contrib/related/git-related\n+++ b/contrib/related/git-related\n@@ -31,10 +31,12 @@ class Person\n @name = name\n @email = email\n @commits = {}\n+ @roles = Hash.new(0)\n end\n \n- def add_role(commit)\n+ def add_role(commit, role)\n @commits[commit] = true\n+ @roles[role] += 1\n end\n \n def <=>(b)\n@@ -79,20 +81,20 @@ class Commit\n end\n \n def parse(data)\n- msg = nil\n+ msg = author = nil\n data.each_line do |line|\n if not msg\n case line\n when /^author ([^<>]+) <(\\S+)> (.+)$/\n author = Persons.get($1, $2)\n- author.add_role(@id)\n+ author.add_role(@id, :author)\n when /^$/\n msg = true\n end\n else\n if line =~ /^(Signed-off|Reviewed|Acked)-by: ([^<>]+) <(\\S+?)>$/\n person = Persons.get($2, $3)\n- person.add_role(@id)\n+ person.add_role(@id, :signer) if person != author\n end\n end\n end\n@@ -174,5 +176,11 @@ persons = Persons.new\n persons.sort.reverse.each do |person|\n percent = person.size.to_f * 100 / commits.size\n next if percent < $min_percent\n- puts '%s (involved: %u%%)' % [person, percent]\n+\n+ roles = person.roles.map do |role, role_count|\n+ role_percent = role_count.to_f * 100 / commits.size\n+ '%s: %u%%' % [role, role_percent]\n+ end\n+\n+ puts '%s (%s)' % [person, roles.join(', ')]\n end\n-- \n1.8.3.rc2.542.g24820ba\n"},{"id":"217803","messageId":"1368877615-9563-8-git-send-email-felipe.contreras@gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 07/15] contrib: related: add support for more roles","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:46:47Z","receivedAt":"2013-05-18T11:46:47Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras \n---\n contrib/related/git-related | 13 +++++++++++--\n 1 file changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/related/git-related b/contrib/related/git-related\nindex cd1ef59..eef776a 100755\n--- a/contrib/related/git-related\n+++ b/contrib/related/git-related\n@@ -23,6 +23,12 @@ begin\n rescue OptionParser::InvalidOption\n end\n \n+KNOWN_ROLES = {\n+ 'Signed-off' => :signer,\n+ 'Reviewed' => :reviewer,\n+ 'Acked' => :acker,\n+}\n+\n class Person\n \n attr_reader :roles\n@@ -92,9 +98,12 @@ class Commit\n msg = true\n end\n else\n- if line =~ /^(Signed-off|Reviewed|Acked)-by: ([^<>]+) <(\\S+?)>$/\n+ role_regex = KNOWN_ROLES.keys.join('|')\n+ if line =~ /^(#{role_regex})-by: ([^<>]+) <(\\S+?)>$/\n person = Persons.get($2, $3)\n- person.add_role(@id, :signer) if person != author\n+ role = KNOWN_ROLES[$1]\n+ next if role == :signer and person == author\n+ person.add_role(@id, role)\n end\n end\n end\n-- \n1.8.3.rc2.542.g24820ba\n"},{"id":"217804","messageId":"1368877615-9563-9-git-send-email-felipe.contreras@gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 08/15] contrib: related: group persons with same email","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:46:48Z","receivedAt":"2013-05-18T11:46:48Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Suggested-by: Duy Nguyen \nSigned-off-by: Felipe Contreras \n---\n contrib/related/git-related | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/related/git-related b/contrib/related/git-related\nindex eef776a..9194777 100755\n--- a/contrib/related/git-related\n+++ b/contrib/related/git-related\n@@ -70,7 +70,7 @@ class Persons\n end\n \n def self.get(name, email)\n- id = [name, email]\n+ id = email.downcase\n person = @@index[id]\n if not person\n person = @@index[id] = Person.new(name, email)\n-- \n1.8.3.rc2.542.g24820ba\n"},{"id":"217805","messageId":"1368877615-9563-10-git-send-email-felipe.contreras@gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 09/15] contrib: related: add mailmap support","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:46:49Z","receivedAt":"2013-05-18T11:46:49Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras \n---\n contrib/related/git-related | 37 +++++++++++++++++++++++++++++++++++++\n 1 file changed, 37 insertions(+)\n\ndiff --git a/contrib/related/git-related b/contrib/related/git-related\nindex 9194777..3b11930 100755\n--- a/contrib/related/git-related\n+++ b/contrib/related/git-related\n@@ -8,6 +8,9 @@ require 'optparse'\n $since = '5-years-ago'\n $min_percent = 10\n \n+$mailmaps = {}\n+$mailmaps_complex = {}\n+\n begin\n OptionParser.new do |opts|\n opts.program_name = 'git related'\n@@ -23,6 +26,29 @@ begin\n rescue OptionParser::InvalidOption\n end\n \n+def get_mailmap(filename)\n+ return unless File.exists?(filename)\n+ File.open(filename) do |f|\n+ f.each do |line|\n+ case line.gsub(/\\s*#.*$/, '')\n+ when /^([^<>]+)\\s+<(\\S+)>$/\n+ $mailmaps[$2] = [ $1, nil ]\n+ when /^<(\\S+)>\\s+<(\\S+)>$/\n+ $mailmaps[$2] = [ nil, $1 ]\n+ when /^([^<>]+)\\s+<(\\S+)>\\s+<(\\S+)>$/\n+ $mailmaps[$3] = [ $1, $2 ]\n+ when /^([^<>]+)\\s+<(\\S+)>\\s+([^<>]+)\\s+<(\\S+)>$/\n+ $mailmaps_complex[[$3, $4]] = [ $1, $2 ]\n+ end\n+ end\n+ end\n+end\n+\n+get_aliases if $get_aliases\n+get_mailmap('.mailmap')\n+mailmap_file = %x[git config mailmap.file].chomp\n+get_mailmap(mailmap_file)\n+\n KNOWN_ROLES = {\n 'Signed-off' => :signer,\n 'Reviewed' => :reviewer,\n@@ -70,6 +96,17 @@ class Persons\n end\n \n def self.get(name, email)\n+\n+ # fix with mailmap\n+ person = [name, email]\n+ new = nil\n+ new = $mailmaps_complex[person] if not new and $mailmaps_complex.include?(person)\n+ new = $mailmaps[email] if not new and $mailmaps.include?(email)\n+ if new\n+ name = new[0] if new[0]\n+ email = new[1] if new[1]\n+ end\n+\n id = email.downcase\n person = @@index[id]\n if not person\n-- \n1.8.3.rc2.542.g24820ba\n"},{"id":"217806","messageId":"1368877615-9563-11-git-send-email-felipe.contreras@gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 10/15] contrib: related: allow usage on other directories","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:46:50Z","receivedAt":"2013-05-18T11:46:50Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Not just the root one (of the project).\n\nSigned-off-by: Felipe Contreras \n---\n contrib/related/git-related | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/related/git-related b/contrib/related/git-related\nindex 3b11930..def2af5 100755\n--- a/contrib/related/git-related\n+++ b/contrib/related/git-related\n@@ -44,8 +44,12 @@ def get_mailmap(filename)\n end\n end\n \n+git_dir = %x[git rev-parse --git-dir].chomp\n+$base_dir = File.dirname(git_dir)\n+$cur_dir = Dir.pwd\n+\n get_aliases if $get_aliases\n-get_mailmap('.mailmap')\n+get_mailmap(File.join($base_dir, '.mailmap'))\n mailmap_file = %x[git config mailmap.file].chomp\n get_mailmap(mailmap_file)\n \n@@ -180,6 +184,7 @@ class Commits\n def get_blame(source, start, len, from)\n return if len == 0\n len ||= 1\n+ Dir.chdir($base_dir)\n File.popen(['git', 'blame', '--incremental', '-C',\n '-L', '%u,+%u' % [start, len],\n '--since', $since, from + '^',\n@@ -191,6 +196,7 @@ class Commits\n end\n end\n end\n+ Dir.chdir($cur_dir)\n end\n \n def from_patch(file)\n-- \n1.8.3.rc2.542.g24820ba\n"},{"id":"217807","messageId":"1368877615-9563-12-git-send-email-felipe.contreras@gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 11/15] contrib: related: add support for multiple patches","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:46:51Z","receivedAt":"2013-05-18T11:46:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras \n---\n contrib/related/git-related | 35 +++++++++++++++++++----------------\n 1 file changed, 19 insertions(+), 16 deletions(-)\n\ndiff --git a/contrib/related/git-related b/contrib/related/git-related\nindex def2af5..a2f98d9 100755\n--- a/contrib/related/git-related\n+++ b/contrib/related/git-related\n@@ -14,7 +14,7 @@ $mailmaps_complex = {}\n begin\n OptionParser.new do |opts|\n opts.program_name = 'git related'\n- opts.banner = 'usage: git related [options] '\n+ opts.banner = 'usage: git related [options] '\n \n opts.on('-p', '--min-percent N', Integer, 'Minium percentage of role participation') do |v|\n $min_percent = v\n@@ -156,6 +156,7 @@ class Commits\n \n def initialize\n @items = {}\n+ @main_commits = {}\n end\n \n def size\n@@ -192,24 +193,28 @@ class Commits\n p.each do |line|\n if line =~ /^(\\h{40})/\n id = $1\n- @items[id] = Commit.new(id)\n+ @items[id] = Commit.new(id) if not @main_commits.include?(id)\n end\n end\n end\n Dir.chdir($cur_dir)\n end\n \n- def from_patch(file)\n- from = source = nil\n- File.open(file) do |f|\n- f.each do |line|\n- case line\n- when /^From (\\h+) (.+)$/\n- from = $1\n- when /^---\\s+(\\S+)/\n- source = $1 != '/dev/null' ? $1[2..-1] : nil\n- when /^@@ -(\\d+)(?:,(\\d+))?/\n- get_blame(source, $1, $2, from)\n+ def from_patches(files)\n+ source = nil\n+ files.each do |file|\n+ from = nil\n+ File.open(file) do |f|\n+ f.each do |line|\n+ case line\n+ when /^From (\\h+) (.+)$/\n+ from = $1\n+ @main_commits[from] = true\n+ when /^---\\s+(\\S+)/\n+ source = $1 != '/dev/null' ? $1[2..-1] : nil\n+ when /^@@ -(\\d+)(?:,(\\d+))?/\n+ get_blame(source, $1, $2, from)\n+ end\n end\n end\n end\n@@ -217,10 +222,8 @@ class Commits\n \n end\n \n-exit 1 if ARGV.size != 1\n-\n commits = Commits.new\n-commits.from_patch(ARGV[0])\n+commits.from_patches(ARGV)\n commits.import\n \n persons = Persons.new\n-- \n1.8.3.rc2.542.g24820ba\n"},{"id":"217808","messageId":"1368877615-9563-13-git-send-email-felipe.contreras@gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 12/15] contrib: related: add option to show commits","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:46:52Z","receivedAt":"2013-05-18T11:46:52Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Instead of showing the authors and signers, show the commits themselves.\n\nSigned-off-by: Felipe Contreras \n---\n contrib/related/git-related | 22 ++++++++++++++++++++++\n 1 file changed, 22 insertions(+)\n\ndiff --git a/contrib/related/git-related b/contrib/related/git-related\nindex a2f98d9..aec156e 100755\n--- a/contrib/related/git-related\n+++ b/contrib/related/git-related\n@@ -7,6 +7,7 @@ require 'optparse'\n \n $since = '5-years-ago'\n $min_percent = 10\n+$show_commits = false\n \n $mailmaps = {}\n $mailmaps_complex = {}\n@@ -22,6 +23,9 @@ begin\n opts.on('-d', '--since DATE', 'How far back to search for relevant commits') do |v|\n $since = v\n end\n+ opts.on('-c', '--commits[=FORMAT]', [:raw, :full], 'List commits instead of persons') do |v|\n+ $show_commits = v || true\n+ end\n end.parse!\n rescue OptionParser::InvalidOption\n end\n@@ -167,6 +171,10 @@ class Commits\n @items.each(&block)\n end\n \n+ def list\n+ @items.keys\n+ end\n+\n def import\n return if @items.empty?\n File.popen(%w[git cat-file --batch], 'r+') do |p|\n@@ -226,6 +234,20 @@ commits = Commits.new\n commits.from_patches(ARGV)\n commits.import\n \n+if $show_commits\n+ cmd = nil\n+ case $show_commits\n+ when :raw\n+ puts commits.list\n+ when :full\n+ cmd = %w[git log --patch --no-walk]\n+ else\n+ cmd = %w[git log --oneline --no-walk]\n+ end\n+ system(*cmd + commits.list) if cmd\n+ exit 0\n+end\n+\n persons = Persons.new\n \n persons.sort.reverse.each do |person|\n-- \n1.8.3.rc2.542.g24820ba\n"},{"id":"217809","messageId":"1368877615-9563-14-git-send-email-felipe.contreras@gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 13/15] contrib: related: add option to parse from committish","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:46:53Z","receivedAt":"2013-05-18T11:46:53Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"For example master..feature-a.\n\nSigned-off-by: Felipe Contreras \n---\n contrib/related/git-related | 36 ++++++++++++++++++++++++++++++++++--\n 1 file changed, 34 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/related/git-related b/contrib/related/git-related\nindex aec156e..8394cdc 100755\n--- a/contrib/related/git-related\n+++ b/contrib/related/git-related\n@@ -8,6 +8,8 @@ require 'optparse'\n $since = '5-years-ago'\n $min_percent = 10\n $show_commits = false\n+$files = []\n+$rev_args = []\n \n $mailmaps = {}\n $mailmaps_complex = {}\n@@ -15,7 +17,7 @@ $mailmaps_complex = {}\n begin\n OptionParser.new do |opts|\n opts.program_name = 'git related'\n- opts.banner = 'usage: git related [options] '\n+ opts.banner = 'usage: git related [options] '\n \n opts.on('-p', '--min-percent N', Integer, 'Minium percentage of role participation') do |v|\n $min_percent = v\n@@ -228,10 +230,40 @@ class Commits\n end\n end\n \n+ def from_rev_args(args)\n+ return if args.empty?\n+ source = nil\n+ File.popen(%w[git rev-list --reverse] + args) do |p|\n+ p.each do |e|\n+ id = e.chomp\n+ @main_commits[id] = true\n+ File.popen(%w[git show -C --oneline] + [id]) do |p|\n+ p.each do |e|\n+ case e\n+ when /^---\\s+(\\S+)/\n+ source = $1 != '/dev/null' ? $1[2..-1] : nil\n+ when /^@@ -(\\d+)(?:,(\\d+))?/\n+ get_blame(source, $1, $2, id)\n+ end\n+ end\n+ end\n+ end\n+ end\n+ end\n+\n+end\n+\n+ARGV.each do |e|\n+ if File.exists?(e)\n+ $files << e\n+ else\n+ $rev_args << e\n+ end\n end\n \n commits = Commits.new\n-commits.from_patches(ARGV)\n+commits.from_patches($files)\n+commits.from_rev_args($rev_args)\n commits.import\n \n if $show_commits\n-- \n1.8.3.rc2.542.g24820ba\n"},{"id":"217810","messageId":"1368877615-9563-15-git-send-email-felipe.contreras@gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 14/15] contrib: related: parse committish like format-patch","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:46:54Z","receivedAt":"2013-05-18T11:46:54Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Signed-off-by: Felipe Contreras \n---\n contrib/related/git-related | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n\ndiff --git a/contrib/related/git-related b/contrib/related/git-related\nindex 8394cdc..62c9b56 100755\n--- a/contrib/related/git-related\n+++ b/contrib/related/git-related\n@@ -232,6 +232,20 @@ class Commits\n \n def from_rev_args(args)\n return if args.empty?\n+\n+ revs = []\n+\n+ File.popen(%w[git rev-parse --revs-only --default=HEAD --symbolic] + args).each do |rev|\n+ revs << rev.chomp\n+ end\n+\n+ case revs.size\n+ when 1\n+ committish = [ '%s..HEAD' % revs[0] ]\n+ else\n+ committish = revs\n+ end\n+\n source = nil\n File.popen(%w[git rev-list --reverse] + args) do |p|\n p.each do |e|\n-- \n1.8.3.rc2.542.g24820ba\n"},{"id":"217811","messageId":"1368877615-9563-16-git-send-email-felipe.contreras@gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v5 15/15] contrib: related: fix parsing of rev-list args","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:46:55Z","receivedAt":"2013-05-18T11:46:55Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"For example '-1'.\n\nSigned-off-by: Felipe Contreras \n---\n contrib/related/git-related | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/related/git-related b/contrib/related/git-related\nindex 62c9b56..69737ac 100755\n--- a/contrib/related/git-related\n+++ b/contrib/related/git-related\n@@ -29,7 +29,8 @@ begin\n $show_commits = v || true\n end\n end.parse!\n-rescue OptionParser::InvalidOption\n+rescue OptionParser::InvalidOption => e\n+ $rev_args += e.args\n end\n \n def get_mailmap(filename)\n@@ -241,9 +242,11 @@ class Commits\n \n case revs.size\n when 1\n- committish = [ '%s..HEAD' % revs[0] ]\n+ r = revs[0]\n+ r = '^' + r if r[0] != '-'\n+ args = [ r, 'HEAD' ]\n else\n- committish = revs\n+ args = revs\n end\n \n source = nil\n-- \n1.8.3.rc2.542.g24820ba\n"},{"id":"217812","messageId":"CAMP44s0QYrzVBsmba1PYMG0yo0CpvTnVm=QHfYif6iu_Zo=tNA@mail.gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 01/15] Add new git-related helper to contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-18T11:55:33Z","receivedAt":"2013-05-18T11:55:33Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sat, May 18, 2013 at 6:46 AM, Felipe Contreras\n wrote:\n\n> contrib/related/git-related | 124 ++++++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 124 insertions(+)\n> create mode 100755 contrib/related/git-related\n\nI tried everything and I don't think it's physically possible to make\nthis script any simpler without severely crippling it's main goal.\n\n-- \nFelipe Contreras\n"},{"id":"217846","messageId":"7vli7bza98.fsf@alter.siamese.dyndns.org","threadId":"33863","inReplyTo":"CAMP44s0QYrzVBsmba1PYMG0yo0CpvTnVm=QHfYif6iu_Zo=tNA@mail.gmail.com","subject":"Re: [PATCH v5 01/15] Add new git-related helper to contrib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-19T07:42:11Z","receivedAt":"2013-05-19T07:42:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Sat, May 18, 2013 at 6:46 AM, Felipe Contreras\n> wrote:\n>\n>> contrib/related/git-related | 124 ++++++++++++++++++++++++++++++++++++++++++++\n>> 1 file changed, 124 insertions(+)\n>> create mode 100755 contrib/related/git-related\n>\n> I tried everything and I don't think it's physically possible to make\n> this script any simpler without severely crippling it's main goal.\n\nHmm, I haven't read these patches yet (I just came back a few hours\nago to a state in which I am well enough to read and write e-mails),\nbut did anybody complain that it is too complex?\n"},{"id":"217854","messageId":"51988d6c4eceb_76e0f31e2080975@nysa.mail","threadId":"33863","inReplyTo":"7vli7bza98.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v5 01/15] Add new git-related helper to contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-19T08:29:32Z","receivedAt":"2013-05-19T08:29:32Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras writes:\n> \n> > On Sat, May 18, 2013 at 6:46 AM, Felipe Contreras\n> > wrote:\n> >\n> >> contrib/related/git-related | 124 ++++++++++++++++++++++++++++++++++++++++++++\n> >> 1 file changed, 124 insertions(+)\n> >> create mode 100755 contrib/related/git-related\n> >\n> > I tried everything and I don't think it's physically possible to make\n> > this script any simpler without severely crippling it's main goal.\n> \n> Hmm, I haven't read these patches yet (I just came back a few hours\n> ago to a state in which I am well enough to read and write e-mails),\n> but did anybody complain that it is too complex?\n\nThis comment was again targetted to Ramkumar, who yes, indeed he complained\nabout it being too complex, at least v3.\n\n-- \nFelipe Contreras\n"},{"id":"217910","messageId":"CALkWK0ke-Ns0JCt6LAzWqympWZ=2x3rRHOjnzB9QN7gc2=ckgw@mail.gmail.com","threadId":"33863","inReplyTo":"1368877615-9563-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v5 01/15] Add new git-related helper to contrib","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-19T14:40:08Z","receivedAt":"2013-05-19T14:40:08Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Okay, let's look at this part.\n\nFelipe Contreras wrote:\n> diff --git a/contrib/related/git-related b/contrib/related/git-related\n> new file mode 100755\n> index 0000000..4f31482\n> --- /dev/null\n> +++ b/contrib/related/git-related\n> @@ -0,0 +1,124 @@\n> +#!/usr/bin/env ruby\n> +\n> +# This script finds people that might be interested in a patch\n> +# usage: git related \n> +\n> +$since = '5-years-ago'\n> +$min_percent = 10\n> +\n> +def fmt_person(name, email)\n> + name ? '%s <%s>' % [name, email] : email\n> +end\n> +\n> +class Commit\n> +\n> + attr_reader :persons\n> +\n> + def initialize(id)\n> + @id = id\n> + @persons = []\n> + end\n\nOkay, although I'm wondering what id is.\n\n> + def parse(data)\n> + msg = nil\n> + data.each_line do |line|\n> + if not msg\n> + case line\n> + when /^author ([^<>]+) <(\\S+)> (.+)$/\n> + @persons << fmt_person($1, $2)\n> + when /^$/\n> + msg = true\n> + end\n\nWhen there's a blank line, flip the switch and start looking in the\ncommit message. Why though? You're worried that the commit message\nwill contain a line matching /^author ([^<>]+) <(\\S+)> (.+)$/? If so,\nwhy don't you split('\\n', 2), look for the author in the $1 and\nWhatevered-by in $2?\n\n> + else\n> + if line =~ /^(Signed-off|Reviewed|Acked)-by: ([^<>]+) <(\\S+?)>$/\n\nelif? Can this be more generic to include Whatevered-by?\n\n> + @persons << fmt_person($2, $3)\n\nWill $2 ever be nil (from fmt_person)? ie. Why are you checking for\nthe special case \" <\\S+?>$\"?\n\n> + end\n> + end\n> + end\n> + @persons.uniq!\n\nSo you don't care if the person has appeared twice or thrice in the message.\n\n> + end\n> +\n> +end\n> +\n> +class Commits\n> +\n> + def initialize\n> + @items = {}\n> + end\n\nOkay.\n\n> + def size\n> + @items.size\n> + end\n\n#size is reminiscent of Array#size and Hash#size.\n\n> + def each(&block)\n> + @items.each(&block)\n> + end\n\nI can see how you iterate over commits from the code below. However,\nusing #each like this is confusing, because the reader expects #each\nto be invoked on an Enumerable. So, I have two suggestions:\n\n1. Use block_given? to make sure that #each works even without a block\n(just like Enumerator#each).\n\n2. Mixin Enumerable by putting in an 'include Enumerable' after the\nclass line. Aside from clarity, you get stuff like #find for free.\n\n> + def import\n\n>From reading the code below, your calling semantics are: first set\neach @items[id] to a new Commit object. import is meant to invoke\nCommit#parse and set @persons there. Okay.\n\n> + return if @items.empty?\n> + File.popen(%w[git cat-file --batch], 'r+') do |p|\n> + p.write(@items.keys.join(\"\\n\"))\n> + p.close_write\n\nOkay.\n\n> + p.each do |l|\n> + if l =~ /^(\\h{40}) commit (\\d+)/\n\ns/l/line/?\n\n> + id, len = $1, $2\n> + data = p.read($2.to_i)\n> + @items[id].parse(data)\n> + end\n> + end\n> + end\n> + end\n> +\n> + def get_blame(source, start, len, from)\n> + return if len == 0\n> + len ||= 1\n\nPlease don't use ||=. It is notorious for causing confusion in\nRuby-land. Hint: it's not exactly equivalent to either len = len || 1\nor len || len = 1.\n\n> + File.popen(['git', 'blame', '--incremental', '-C',\n\nStill no -CCC?\n\n> + '-L', '%u,+%u' % [start, len],\n> + '--since', $since, from + '^',\n> + '--', source]) do |p|\n> + p.each do |line|\n> + if line =~ /^(\\h{40})/\n> + id = $1\n\nUse $0 and remove the parens: you're matching the whole line.\n\n> + @items[id] = Commit.new(id)\n\nOkay.\n\n> + end\n> + end\n> + end\n> + end\n> +\n> + def from_patch(file)\n> + from = source = nil\n> + File.open(file) do |f|\n> + f.each do |line|\n> + case line\n> + when /^From (\\h+) (.+)$/\n> + from = $1\n\nOkay.\n\n> + when /^---\\s+(\\S+)/\n> + source = $1 != '/dev/null' ? $1[2..-1] : nil\n\nOkay.\n\n> + when /^@@ -(\\d+)(?:,(\\d+))?/\n> + get_blame(source, $1, $2, from)\n\nOkay.\n\n> + end\n> + end\n> + end\n> + end\n> +\n> +end\n> +\n> +exit 1 if ARGV.size != 1\n\nOkay.\n\n> +commits = Commits.new\n> +commits.from_patch(ARGV[0])\n> +commits.import\n\nThe calling semantics could be better, but it's not a big complaint.\n\n> +count_per_person = Hash.new(0)\n\nInitializing all keys to 0. Okay.\n\n> +commits.each do |id, commit|\n\nCute.\n\n> + commit.persons.each do |person|\n> + count_per_person[person] += 1\n\nOkay.\n\n> + end\n> +end\n> +\n> +count_per_person.each do |person, count|\n> + percent = count.to_f * 100 / commits.size\n> + next if percent < $min_percent\n> + puts person\n\nNot going to print percentage as well?\n\nOverall, significant improvement over v3 which used all kinds of\nunnecessarily complex data structures and convoluted logic. Looks\nlike something I'd want to use: not blindly as a cc-cmd, but just to\nget a quick idea. I also wish for depth most times: a working\nshortlog -L --follow would be really nice.\n"},{"id":"217911","messageId":"CAMP44s1xguzX=iavUJxYXBEPOpGvJtJ_7FDHNCWMrF6qxOsuOQ@mail.gmail.com","threadId":"33863","inReplyTo":"CALkWK0ke-Ns0JCt6LAzWqympWZ=2x3rRHOjnzB9QN7gc2=ckgw@mail.gmail.com","subject":"Re: [PATCH v5 01/15] Add new git-related helper to contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-19T15:05:39Z","receivedAt":"2013-05-19T15:05:39Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, May 19, 2013 at 9:40 AM, Ramkumar Ramachandra\n wrote:\n> Okay, let's look at this part.\n>\n> Felipe Contreras wrote:\n>> diff --git a/contrib/related/git-related b/contrib/related/git-related\n>> new file mode 100755\n>> index 0000000..4f31482\n>> --- /dev/null\n>> +++ b/contrib/related/git-related\n>> @@ -0,0 +1,124 @@\n>> +#!/usr/bin/env ruby\n>> +\n>> +# This script finds people that might be interested in a patch\n>> +# usage: git related \n>> +\n>> +$since = '5-years-ago'\n>> +$min_percent = 10\n>> +\n>> +def fmt_person(name, email)\n>> + name ? '%s <%s>' % [name, email] : email\n>> +end\n>> +\n>> +class Commit\n>> +\n>> + attr_reader :persons\n>> +\n>> + def initialize(id)\n>> + @id = id\n>> + @persons = []\n>> + end\n>\n> Okay, although I'm wondering what id is.\n\nThe commit's unique identifier. How is that not clear?\n\n>> + def parse(data)\n>> + msg = nil\n>> + data.each_line do |line|\n>> + if not msg\n>> + case line\n>> + when /^author ([^<>]+) <(\\S+)> (.+)$/\n>> + @persons << fmt_person($1, $2)\n>> + when /^$/\n>> + msg = true\n>> + end\n>\n> When there's a blank line, flip the switch and start looking in the\n> commit message. Why though? You're worried that the commit message\n> will contain a line matching /^author ([^<>]+) <(\\S+)> (.+)$/? If so,\n> why don't you split('\\n', 2), look for the author in the $1 and\n> Whatevered-by in $2?\n\nThat's not efficient, it's more efficient to parse line by line\nlazily, and it's not that complicated.\n\n>> + else\n>> + if line =~ /^(Signed-off|Reviewed|Acked)-by: ([^<>]+) <(\\S+?)>$/\n>\n> elif?\n\nIt will get bigger soon enough.\n\n> Can this be more generic to include Whatevered-by?\n\nThat's done in later patches. This is a minimal version.\n\n>> + @persons << fmt_person($2, $3)\n>\n> Will $2 ever be nil (from fmt_person)? ie. Why are you checking for\n> the special case \" <\\S+?>$\"?\n\nYes, '' was valid in earlier versions of git.\n\n>> + end\n>> + end\n>> + end\n>> + @persons.uniq!\n>\n> So you don't care if the person has appeared twice or thrice in the message.\n\nYes I do, otherwise they be counted multiple times, and their\nparticipation calculation would go beyond 100%.\n\n>> + def size\n>> + @items.size\n>> + end\n>\n> #size is reminiscent of Array#size and Hash#size.\n\nPrecisely. I would even include Array if it made sense.\n\n>> + def each(&block)\n>> + @items.each(&block)\n>> + end\n>\n> I can see how you iterate over commits from the code below. However,\n> using #each like this is confusing, because the reader expects #each\n> to be invoked on an Enumerable. So, I have two suggestions:\n\nWe can include Enumerable, it won't make a difference.\n\n> 1. Use block_given? to make sure that #each works even without a block\n> (just like Enumerator#each).\n\nIt already works just like Enumerator#each; commits.each returns an\nEnumerator because @items.each returns an Enumerator.\n\n> 2. Mixin Enumerable by putting in an 'include Enumerable' after the\n> class line. Aside from clarity, you get stuff like #find for free.\n\nStuff we don't need or want.\n\n>> + def import\n>\n> From reading the code below, your calling semantics are: first set\n> each @items[id] to a new Commit object. import is meant to invoke\n> Commit#parse and set @persons there. Okay.\n>\n>> + return if @items.empty?\n>> + File.popen(%w[git cat-file --batch], 'r+') do |p|\n>> + p.write(@items.keys.join(\"\\n\"))\n>> + p.close_write\n>\n> Okay.\n>\n>> + p.each do |l|\n>> + if l =~ /^(\\h{40}) commit (\\d+)/\n>\n> s/l/line/?\n\nFine.\n\n>> + id, len = $1, $2\n>> + data = p.read($2.to_i)\n>> + @items[id].parse(data)\n>> + end\n>> + end\n>> + end\n>> + end\n>> +\n>> + def get_blame(source, start, len, from)\n>> + return if len == 0\n>> + len ||= 1\n>\n> Please don't use ||=. It is notorious for causing confusion in\n> Ruby-land. Hint: it's not exactly equivalent to either len = len || 1\n> or len || len = 1.\n\nHow exactly is it not equivalent to len = len || 1?\n\nThis is what I want:\n\nlen = 1 if not len\n\nAnd I think the above does exactly that.\n\n>> + File.popen(['git', 'blame', '--incremental', '-C',\n>\n> Still no -CCC?\n\nI forgot.\n\n>> + '-L', '%u,+%u' % [start, len],\n>> + '--since', $since, from + '^',\n>> + '--', source]) do |p|\n>> + p.each do |line|\n>> + if line =~ /^(\\h{40})/\n>> + id = $1\n>\n> Use $0 and remove the parens: you're matching the whole line.\n\nNo, I'm not matching the whole line, but you are right; there's no\nneed for groups.\n\n>> + end\n>> +end\n>> +\n>> +count_per_person.each do |person, count|\n>> + percent = count.to_f * 100 / commits.size\n>> + next if percent < $min_percent\n>> + puts person\n>\n> Not going to print percentage as well?\n\nLater. This does the minimum.\n\n> Overall, significant improvement over v3 which used all kinds of\n> unnecessarily complex data structures and convoluted logic.\n\nIt's coming.\n\n-- \nFelipe Contreras\n"},{"id":"217912","messageId":"CALkWK0kz-pQ9UJPXerFoui9ftNXDzD-xhpkSHY=JX3rbFXgp-g@mail.gmail.com","threadId":"33863","inReplyTo":"CAMP44s1xguzX=iavUJxYXBEPOpGvJtJ_7FDHNCWMrF6qxOsuOQ@mail.gmail.com","subject":"Re: [PATCH v5 01/15] Add new git-related helper to contrib","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-19T15:13:27Z","receivedAt":"2013-05-19T15:13:27Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> How exactly is it not equivalent to len = len || 1?\n\nHere, I dug up an article for you on the issue:\n\nhttp://www.rubyinside.com/what-rubys-double-pipe-or-equals-really-does-5488.html\n\nAlthough it's fine in this case, I wouldn't recommend using ||=\nbecause of the potential confusion.\n"},{"id":"217913","messageId":"CALkWK0mA9zWjco1fYRu4O2QmMRJE7vd2Z3CyJj=a1VxCyymMvQ@mail.gmail.com","threadId":"33863","inReplyTo":"CAMP44s1xguzX=iavUJxYXBEPOpGvJtJ_7FDHNCWMrF6qxOsuOQ@mail.gmail.com","subject":"Re: [PATCH v5 01/15] Add new git-related helper to contrib","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-19T15:15:28Z","receivedAt":"2013-05-19T15:15:28Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n>> Will $2 ever be nil (from fmt_person)? ie. Why are you checking for\n>> the special case \" <\\S+?>$\"?\n>\n> Yes, '' was valid in earlier versions of git.\n\nThere's a non-optional space before the \"\" in your regex, which\nis what I was pointing out.\n"},{"id":"217914","messageId":"CALkWK0kuj00uUW5C5zKyaCBM72HBzxAk9G2UT1P3bKx9Lc8tkg@mail.gmail.com","threadId":"33863","inReplyTo":"CALkWK0mA9zWjco1fYRu4O2QmMRJE7vd2Z3CyJj=a1VxCyymMvQ@mail.gmail.com","subject":"Re: [PATCH v5 01/15] Add new git-related helper to contrib","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-19T15:17:32Z","receivedAt":"2013-05-19T15:17:32Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> There's a non-optional space before the \"\" in your regex, which\n> is what I was pointing out.\n\nEr, scratch that. It's the space after the \"Whatevered-by:\"\n"},{"id":"217915","messageId":"CAMP44s3idD9pen0PyFuHeXQ=uphA8gVfFCEYhEnDRsx-gszbAw@mail.gmail.com","threadId":"33863","inReplyTo":"CALkWK0kz-pQ9UJPXerFoui9ftNXDzD-xhpkSHY=JX3rbFXgp-g@mail.gmail.com","subject":"Re: [PATCH v5 01/15] Add new git-related helper to contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-19T15:41:57Z","receivedAt":"2013-05-19T15:41:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, May 19, 2013 at 10:13 AM, Ramkumar Ramachandra\n wrote:\n> Felipe Contreras wrote:\n>> How exactly is it not equivalent to len = len || 1?\n>\n> Here, I dug up an article for you on the issue:\n>\n> http://www.rubyinside.com/what-rubys-double-pipe-or-equals-really-does-5488.html\n>\n> Although it's fine in this case, I wouldn't recommend using ||=\n> because of the potential confusion.\n\nI don't see the confusion, 'len ||= 1' is *exactly* the same as 'len =\n1 if not len', which is what I expected.\n\n-- \nFelipe Contreras\n"},{"id":"217916","messageId":"CAMP44s2v5Z=7cJ4-3WeqVHAqxk1sWVT-VkOufK5CeCVcS8Xh8g@mail.gmail.com","threadId":"33863","inReplyTo":"CALkWK0kuj00uUW5C5zKyaCBM72HBzxAk9G2UT1P3bKx9Lc8tkg@mail.gmail.com","subject":"Re: [PATCH v5 01/15] Add new git-related helper to contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-19T15:45:58Z","receivedAt":"2013-05-19T15:45:58Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, May 19, 2013 at 10:17 AM, Ramkumar Ramachandra\n wrote:\n> Ramkumar Ramachandra wrote:\n>> There's a non-optional space before the \"\" in your regex, which\n>> is what I was pointing out.\n>\n> Er, scratch that. It's the space after the \"Whatevered-by:\"\n\nIt doesn't really matter. We can operate under the assumption that\nthere will always be a name and remove the extra code in fmt_person(),\nIIRC it was because of the muttrc simplification which I don't think\nmakes sense any more. It's still possible that the name will be\nmissing, but I don't personally care, it would probably work for\n99.99% of the cases.\n\n-- \nFelipe Contreras\n"},{"id":"217917","messageId":"CAMP44s34uDwZiFiy7cno_3M+_QB2zweGsuwvpYQ6p4O_Ct3CZA@mail.gmail.com","threadId":"33863","inReplyTo":"CAMP44s1xguzX=iavUJxYXBEPOpGvJtJ_7FDHNCWMrF6qxOsuOQ@mail.gmail.com","subject":"Re: [PATCH v5 01/15] Add new git-related helper to contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-19T15:48:57Z","receivedAt":"2013-05-19T15:48:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Sun, May 19, 2013 at 10:05 AM, Felipe Contreras\n wrote:\n> On Sun, May 19, 2013 at 9:40 AM, Ramkumar Ramachandra\n> wrote:\n\n>>> + '-L', '%u,+%u' % [start, len],\n>>> + '--since', $since, from + '^',\n>>> + '--', source]) do |p|\n>>> + p.each do |line|\n>>> + if line =~ /^(\\h{40})/\n>>> + id = $1\n>>\n>> Use $0 and remove the parens: you're matching the whole line.\n>\n> No, I'm not matching the whole line, but you are right; there's no\n> need for groups.\n\nActually $&.\n\n-- \nFelipe Contreras\n"},{"id":"217946","messageId":"CAJDDKr7+L1Qa9419aLabvnVPFCeLh0CZR9Vdazd8uk0biZxu1g@mail.gmail.com","threadId":"33863","inReplyTo":"CAMP44s3idD9pen0PyFuHeXQ=uphA8gVfFCEYhEnDRsx-gszbAw@mail.gmail.com","subject":"Re: [PATCH v5 01/15] Add new git-related helper to contrib","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2013-05-19T21:14:18Z","receivedAt":"2013-05-19T21:14:18Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sun, May 19, 2013 at 8:41 AM, Felipe Contreras\n wrote:\n> On Sun, May 19, 2013 at 10:13 AM, Ramkumar Ramachandra\n> wrote:\n>> Felipe Contreras wrote:\n>>> How exactly is it not equivalent to len = len || 1?\n>>\n>> Here, I dug up an article for you on the issue:\n>>\n>> http://www.rubyinside.com/what-rubys-double-pipe-or-equals-really-does-5488.html\n>>\n>> Although it's fine in this case, I wouldn't recommend using ||=\n>> because of the potential confusion.\n>\n> I don't see the confusion, 'len ||= 1' is *exactly* the same as 'len =\n> 1 if not len', which is what I expected.\n\nI spend more time in Python, C*, and Perl then in Ruby, and I was not\nconfused by this at all.\n\nIt behaves just like $foo ||= 1 does in Perl.\n\nIn git.git:\n\n$ git grep '||=' | wc -l\n 121\n--\nDavid\n"}]}