{"thread":{"id":"33541","subject":"[PATCH v2 0/8] New git-cc-cmd helper","startedAt":"2013-04-19T05:14:10Z","lastAt":"2013-04-22T07:30:30Z","messageCount":22,"participants":["Felipe Contreras","Ramkumar Ramachandra","Junio C Hamano","Johannes Sixt","Jeremy Rosen"],"isPatch":true,"patchVersion":2,"patchTotal":8},"messages":[{"id":"214796","messageId":"1366348458-7706-1-git-send-email-felipe.contreras@gmail.com","threadId":"33541","inReplyTo":null,"subject":"[PATCH v2 0/8] New git-cc-cmd helper","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-19T05:14:10Z","receivedAt":"2013-04-19T05:14:10Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Hi,\n\nThis script allows you to get a list of relevant persons to Cc when sending a\npatch series.\n\n % git cc-cmd v1.8.1.6^^1..v1.8.1.6^^2\n \"Henrik Grubbström\" (author: 7%)\n junio (signer: 84%, author: 15%)\n \"Nguyễn Thái Ngọc Duy\" (author: 30%, signer: 7%)\n \"Jean-Noël AVILA\" (author: 7%)\n Jean-Noel Avila (signer: 7%)\n Duy Nguyen (author: 7%)\n Michael Haggerty (author: 15%)\n Clemens Buchacher (author: 7%)\n Joshua Jensen (author: 7%)\n Johannes Sixt (signer: 7%)\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 cc-cmd --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 407a963 Merge branch 'rr/remote-helper-doc'\n ec775c4 attr: Expand macros immediately when encountered.\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\nFelipe Contreras (8):\n Add new git-cc-cmd helper to contrib\n contrib: cc-cmd: add option parsing\n contrib: cc-cmd: add support for multiple patches\n contrib: cc-cmd: add option to show commits\n contrib: cc-cmd: add option to parse from committish\n contrib: cc-cmd: parse committish like format-patch\n contrib: cc-cmd: fix parsing of rev-list args\n contrib: cc-cmd: add option to fetch aliases\n\n contrib/cc-cmd/git-cc-cmd | 245 ++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 245 insertions(+)\n create mode 100755 contrib/cc-cmd/git-cc-cmd\n\n-- \n1.8.2.1.790.g4588561\n"},{"id":"214797","messageId":"1366348458-7706-2-git-send-email-felipe.contreras@gmail.com","threadId":"33541","inReplyTo":"1366348458-7706-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 1/8] Add new git-cc-cmd helper to contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-19T05:14:11Z","receivedAt":"2013-04-19T05:14:11Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"The code finds the changes of a commit, runs 'git blame' for each chunk\nto see which other commits are relevant, and then reports the author and\nsigners.\n\nFinally, it calculates what percentage of the total relevant commits\neach person was involved in, and show only the ones that pass the\nthreshold.\n\nFor example:\n\n % git cc-cmd 0001-remote-hg-trivial-cleanups.patch\n Felipe Contreras (author: 100%)\n Jeff King (signer: 83%)\n Max Horn (signer: 16%)\n Junio C Hamano (signer: 16%)\n\nThus it can be used for 'git send-email' as a cc-cmd.\n\nSigned-off-by: Felipe Contreras \n---\n contrib/cc-cmd/git-cc-cmd | 140 ++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 140 insertions(+)\n create mode 100755 contrib/cc-cmd/git-cc-cmd\n\ndiff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd\nnew file mode 100755\nindex 0000000..c7ecf79\n--- /dev/null\n+++ b/contrib/cc-cmd/git-cc-cmd\n@@ -0,0 +1,140 @@\n+#!/usr/bin/env ruby\n+\n+$since = '3-years-ago'\n+$min_percent = 5\n+\n+class Commit\n+\n+ attr_reader :id\n+ attr_accessor :roles\n+\n+ def initialize(id)\n+ @id = id\n+ @roles = []\n+ end\n+\n+ def self.parse(data)\n+ id = author = msg = nil\n+ roles = {}\n+ data.each_line do |line|\n+ if not msg\n+ case line\n+ when /^commit (.+)$/\n+ id = $1\n+ when /^author ([^<>]+) <(\\S+)>$/\n+ author = $1, $2\n+ roles[author] = 'author'\n+ when /^$/\n+ msg = true\n+ end\n+ else\n+ if line =~ /^(Signed-off|Reviewed|Acked)-by: ([^<>]+) <(\\S+?)>$/\n+ person = $2, $3\n+ roles[person] = 'signer' if person != author\n+ end\n+ end\n+ end\n+ roles = roles.map do |person, role|\n+ address = \"%s <%s>\" % person\n+ [person, role]\n+ end\n+ [id, roles]\n+ end\n+\n+end\n+\n+class Commits\n+\n+ attr_reader :items\n+\n+ def initialize()\n+ @items = {}\n+ end\n+\n+ def size\n+ @items.size\n+ end\n+\n+ def import\n+ return if @items.empty?\n+ format = [ 'commit %H', 'author %an <%ae>', '', '%B' ].join('%n')\n+ File.popen(['git', 'show', '-z', '-s', '--format=format:' + format] + @items.keys) do |p|\n+ p.each(\"\\0\") do |data|\n+ next if data == \"\\0\" # bug in git show?\n+ id, roles = Commit.parse(data)\n+ commit = @items[id]\n+ commit.roles = roles\n+ end\n+ end\n+ end\n+\n+ def each_person_role\n+ commit_roles = @items.values.map { |commit| commit.roles }.flatten(1)\n+ commit_roles.group_by { |person, role| person }.each do |person, commit_roles|\n+ commit_roles.group_by { |person, role| role }.each do |role, commit_roles|\n+ yield person, role, commit_roles.size\n+ end\n+ end\n+ end\n+\n+ def get_blame(source, start, offset, from)\n+ return unless source\n+ File.popen(['git', 'blame', '--incremental', '-C',\n+ '-L', '%u,+%u' % [start, offset],\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+ source = nil\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+ when /^---\\s+(\\S+)/\n+ source = $1 != '/dev/null' ? $1[2..-1] : nil\n+ when /^@@\\s-(\\d+),(\\d+)/\n+ get_blame(source, $1, $2, from)\n+ end\n+ end\n+ end\n+ import\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+\n+# hash of hashes\n+persons = Hash.new { |hash, key| hash[key] = {} }\n+\n+commits.each_person_role do |person, role, count|\n+ persons[person][role] = count\n+end\n+\n+persons.each do |person, roles|\n+ roles = roles.map do |role, count|\n+ percent = count.to_f * 100 / commits.size\n+ next if percent < $min_percent\n+ \"%s: %u%%\" % [role, percent]\n+ end.compact\n+ next if roles.empty?\n+\n+ name, email = person\n+ # must quote chars?\n+ name = '\"%s\"' % name if name =~ /[^\\w \\-]/i\n+ person = name ? \"%s <%s>\" % [name, email] : email\n+ puts \"%s (%s)\" % [person, roles.join(', ')]\n+end\n-- \n1.8.2.1.790.g4588561\n"},{"id":"214798","messageId":"1366348458-7706-3-git-send-email-felipe.contreras@gmail.com","threadId":"33541","inReplyTo":"1366348458-7706-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 2/8] contrib: cc-cmd: add option parsing","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-19T05:14:12Z","receivedAt":"2013-04-19T05:14:12Z","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/cc-cmd/git-cc-cmd | 21 +++++++++++++++++++--\n 1 file changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd\nindex c7ecf79..0a5ec01 100755\n--- a/contrib/cc-cmd/git-cc-cmd\n+++ b/contrib/cc-cmd/git-cc-cmd\n@@ -1,8 +1,25 @@\n #!/usr/bin/env ruby\n \n+require 'optparse'\n+\n $since = '3-years-ago'\n $min_percent = 5\n \n+begin\n+ OptionParser.new do |opts|\n+ opts.program_name = 'git cc-cmd'\n+ opts.banner = 'usage: git cc-cmd [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 :id\n@@ -107,15 +124,15 @@ class Commits\n end\n end\n end\n- import\n end\n \n end\n \n exit 1 if ARGV.size != 1\n \n-commits = Commits.new()\n+commits = Commits.new\n commits.from_patch(ARGV[0])\n+commits.import\n \n # hash of hashes\n persons = Hash.new { |hash, key| hash[key] = {} }\n-- \n1.8.2.1.790.g4588561\n"},{"id":"214799","messageId":"1366348458-7706-4-git-send-email-felipe.contreras@gmail.com","threadId":"33541","inReplyTo":"1366348458-7706-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 3/8] contrib: cc-cmd: add support for multiple patches","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-19T05:14:13Z","receivedAt":"2013-04-19T05:14:13Z","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/cc-cmd/git-cc-cmd | 34 ++++++++++++++++++----------------\n 1 file changed, 18 insertions(+), 16 deletions(-)\n\ndiff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd\nindex 0a5ec01..e36b1bf 100755\n--- a/contrib/cc-cmd/git-cc-cmd\n+++ b/contrib/cc-cmd/git-cc-cmd\n@@ -8,7 +8,7 @@ $min_percent = 5\n begin\n OptionParser.new do |opts|\n opts.program_name = 'git cc-cmd'\n- opts.banner = 'usage: git cc-cmd [options] '\n+ opts.banner = 'usage: git cc-cmd [options] '\n \n opts.on('-p', '--min-percent N', Integer, 'Minium percentage of role participation') do |v|\n $min_percent = v\n@@ -66,6 +66,7 @@ class Commits\n \n def initialize()\n @items = {}\n+ @main_commits = {}\n end\n \n def size\n@@ -103,24 +104,27 @@ 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 end\n \n- def from_patch(file)\n+ def from_patches(files)\n source = nil\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- when /^---\\s+(\\S+)/\n- source = $1 != '/dev/null' ? $1[2..-1] : nil\n- when /^@@\\s-(\\d+),(\\d+)/\n- get_blame(source, $1, $2, from)\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 /^@@\\s-(\\d+),(\\d+)/\n+ get_blame(source, $1, $2, from)\n+ end\n end\n end\n end\n@@ -128,10 +132,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 # hash of hashes\n-- \n1.8.2.1.790.g4588561\n"},{"id":"214803","messageId":"1366348458-7706-5-git-send-email-felipe.contreras@gmail.com","threadId":"33541","inReplyTo":"1366348458-7706-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 4/8] contrib: cc-cmd: add option to show commits","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-19T05:14:14Z","receivedAt":"2013-04-19T05:14:14Z","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/cc-cmd/git-cc-cmd | 13 +++++++++++++\n 1 file changed, 13 insertions(+)\n\ndiff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd\nindex e36b1bf..f13ed8f 100755\n--- a/contrib/cc-cmd/git-cc-cmd\n+++ b/contrib/cc-cmd/git-cc-cmd\n@@ -4,6 +4,7 @@ require 'optparse'\n \n $since = '3-years-ago'\n $min_percent = 5\n+$show_commits = false\n \n begin\n OptionParser.new do |opts|\n@@ -16,6 +17,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], 'List commits instead of persons') do |v|\n+ $show_commits = v || true\n+ end\n end.parse!\n rescue OptionParser::InvalidOption\n end\n@@ -136,6 +140,15 @@ commits = Commits.new\n commits.from_patches(ARGV)\n commits.import\n \n+if $show_commits\n+ if $show_commits == :raw\n+ puts commits.items.keys\n+ else\n+ system(*['git', 'log', '--oneline', '--no-walk'] + commits.items.keys)\n+ end\n+ exit 0\n+end\n+\n # hash of hashes\n persons = Hash.new { |hash, key| hash[key] = {} }\n \n-- \n1.8.2.1.790.g4588561\n"},{"id":"214804","messageId":"1366348458-7706-6-git-send-email-felipe.contreras@gmail.com","threadId":"33541","inReplyTo":"1366348458-7706-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 5/8] contrib: cc-cmd: add option to parse from committish","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-19T05:14:15Z","receivedAt":"2013-04-19T05:14:15Z","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/cc-cmd/git-cc-cmd | 36 ++++++++++++++++++++++++++++++++++--\n 1 file changed, 34 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd\nindex f13ed8f..462f22c 100755\n--- a/contrib/cc-cmd/git-cc-cmd\n+++ b/contrib/cc-cmd/git-cc-cmd\n@@ -5,11 +5,13 @@ require 'optparse'\n $since = '3-years-ago'\n $min_percent = 5\n $show_commits = false\n+$files = []\n+$rev_args = []\n \n begin\n OptionParser.new do |opts|\n opts.program_name = 'git cc-cmd'\n- opts.banner = 'usage: git cc-cmd [options] '\n+ opts.banner = 'usage: git cc-cmd [options] '\n \n opts.on('-p', '--min-percent N', Integer, 'Minium percentage of role participation') do |v|\n $min_percent = v\n@@ -134,10 +136,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 --no-pager 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 /^@@\\s-(\\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.2.1.790.g4588561\n"},{"id":"214800","messageId":"1366348458-7706-7-git-send-email-felipe.contreras@gmail.com","threadId":"33541","inReplyTo":"1366348458-7706-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 6/8] contrib: cc-cmd: parse committish like format-patch","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-19T05:14:16Z","receivedAt":"2013-04-19T05:14:16Z","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/cc-cmd/git-cc-cmd | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n\ndiff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd\nindex 462f22c..02e1a99 100755\n--- a/contrib/cc-cmd/git-cc-cmd\n+++ b/contrib/cc-cmd/git-cc-cmd\n@@ -138,6 +138,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.2.1.790.g4588561\n"},{"id":"214802","messageId":"1366348458-7706-8-git-send-email-felipe.contreras@gmail.com","threadId":"33541","inReplyTo":"1366348458-7706-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 7/8] contrib: cc-cmd: fix parsing of rev-list args","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-19T05:14:17Z","receivedAt":"2013-04-19T05:14:17Z","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/cc-cmd/git-cc-cmd | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd\nindex 02e1a99..6911259 100755\n--- a/contrib/cc-cmd/git-cc-cmd\n+++ b/contrib/cc-cmd/git-cc-cmd\n@@ -23,7 +23,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 class Commit\n@@ -147,9 +148,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.2.1.790.g4588561\n"},{"id":"214801","messageId":"1366348458-7706-9-git-send-email-felipe.contreras@gmail.com","threadId":"33541","inReplyTo":"1366348458-7706-1-git-send-email-felipe.contreras@gmail.com","subject":"[PATCH v2 8/8] contrib: cc-cmd: add option to fetch aliases","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-19T05:14:18Z","receivedAt":"2013-04-19T05:14:18Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Only the mutt format is supported for now.\n\nSigned-off-by: Felipe Contreras \n---\n contrib/cc-cmd/git-cc-cmd | 24 ++++++++++++++++++++++++\n 1 file changed, 24 insertions(+)\n\ndiff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd\nindex 6911259..02548c6 100755\n--- a/contrib/cc-cmd/git-cc-cmd\n+++ b/contrib/cc-cmd/git-cc-cmd\n@@ -7,6 +7,8 @@ $min_percent = 5\n $show_commits = false\n $files = []\n $rev_args = []\n+$get_aliases = false\n+$aliases = {}\n \n begin\n OptionParser.new do |opts|\n@@ -22,11 +24,32 @@ begin\n opts.on('-c', '--commits[=FORMAT]', [:raw], 'List commits instead of persons') do |v|\n $show_commits = v || true\n end\n+ opts.on('-a', '--aliases', 'Use aliases') do |v|\n+ $get_aliases = v\n+ end\n end.parse!\n rescue OptionParser::InvalidOption => e\n $rev_args += e.args\n end\n \n+def get_aliases\n+ type = %x[git config sendemail.aliasfiletype].chomp\n+ return if type != 'mutt'\n+ file = %x[git config sendemail.aliasesfile].chomp\n+ File.open(File.expand_path(file)) do |f|\n+ f.each do |line|\n+ if line =~ /^\\s*alias\\s+(?:-group\\s+\\S+\\s+)*(\\S+)\\s+(.*)$/\n+ key, addresses = $1, $2.split(', ')\n+ addresses.each do |address|\n+ $aliases[address] = key\n+ end\n+ end\n+ end\n+ end\n+end\n+\n+get_aliases if $get_aliases\n+\n class Commit\n \n attr_reader :id\n@@ -60,6 +83,7 @@ class Commit\n end\n roles = roles.map do |person, role|\n address = \"%s <%s>\" % person\n+ person = nil, $aliases[address] if $aliases.include?(address)\n [person, role]\n end\n [id, roles]\n-- \n1.8.2.1.790.g4588561\n"},{"id":"214830","messageId":"CALkWK0n+cWspBJtH7JFvsvoeHHuiWR0bGpYVU63e7O74XY=4sQ@mail.gmail.com","threadId":"33541","inReplyTo":"1366348458-7706-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 1/8] Add new git-cc-cmd helper to contrib","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-04-19T13:26:34Z","receivedAt":"2013-04-19T13:26:34Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> The code finds the changes of a commit, runs 'git blame' for each chunk\n> to see which other commits are relevant, and then reports the author and\n> signers.\n>\n> Finally, it calculates what percentage of the total relevant commits\n> each person was involved in, and show only the ones that pass the\n> threshold.\n\nUm, this didn't really explain it to me. How about:\n\nThis command takes a patch prepared by 'git format-patch' as an\nargument, and runs 'git blame' on every hunk of the diff to determine\nthe commits that added/removed those lines. It then aggregates the\nauthors and signers of all the commits, and prints out these people in\ndescending order of the percentage of commits they were responsible\nfor. It omits people are below a certain threshold.\n\n> % git cc-cmd 0001-remote-hg-trivial-cleanups.patch\n> Felipe Contreras (author: 100%)\n> Jeff King (signer: 83%)\n> Max Horn (signer: 16%)\n> Junio C Hamano (signer: 16%)\n\nWon't my name appear as the first one on each of my patches? Will I\nbe CC'ed on every patch that I send out?\n\n> contrib/cc-cmd/git-cc-cmd | 140 ++++++++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 140 insertions(+)\n> create mode 100755 contrib/cc-cmd/git-cc-cmd\n\nNo README?\n\n> diff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd\n> new file mode 100755\n> index 0000000..c7ecf79\n> --- /dev/null\n> +++ b/contrib/cc-cmd/git-cc-cmd\n> @@ -0,0 +1,140 @@\n> +#!/usr/bin/env ruby\n\nWhat is the minimum required version of Ruby, by the way? I see you\nhaven't used any fancy lazy enumerators or optional arguments\nintroduced in 2.0, so we should be okay practically speaking.\n\n> +$since = '3-years-ago'\n> +$min_percent = 5\n\nDo I get to configure these two? Also, the '$' prefix from your Perl\nhabit? It's not idiomatic in Ruby afaik.\n\n> +class Commit\n> +\n> + attr_reader :id\n> + attr_accessor :roles\n> +\n> + def initialize(id)\n> + @id = id\n> + @roles = []\n\nWhat are id, roles exactly? This isn't C where we have types, so\nyou're going to have to use some (rdoc-parseable) comments, if you\nwant me to be able to read the code without jumping around too much.\n\n> + end\n> +\n> + def self.parse(data)\n> + id = author = msg = nil\n\nThis is not C. You don't need to initialize stuff, unless you're\ngoing to start accessing it using a .append() or similar. In that\ncase, you need to initialize it as an empty list, so the compiler\nknows what you're appending to.\n\n> + roles = {}\n\nShouldn't this be @roles? Didn't you initialize it as an empty list\nin initialize()? Why did you suddenly change your mind and make it a\nhashtable now?\n\n> + data.each_line do |line|\n> + if not msg\n> + case line\n> + when /^commit (.+)$/\n> + id = $1\n\nOkay, I have no idea what you're parsing yet. There's some commit\nline, so I know this is not a raw commit object, as the name of the\nclass Commit would've indicated.\n\n> + when /^author ([^<>]+) <(\\S+)>$/\n> + author = $1, $2\n> + roles[author] = 'author'\n\nHuh? the key is a two-item list, and the value is a hard-coded\n'author' string? If it's meant to be a sematic value, use a symbol.\n\nAlso, why don't you just collect the authors and signers in two\nseparate lists, instead of doing this and burdening yourself with\naccumulation later?\n\n> + when /^$/\n> + msg = true\n\nI can't see msg being used later in the function, so I don't know what\nyou're doing.\n\n> + roles = roles.map do |person, role|\n> + address = \"%s <%s>\" % person\n> + [person, role]\n\nWhat?! If you wanted address in the first place, why did you stuff a\ntwo-member list into roles as the key? Where is address being used?\n\n> + def import\n> + return if @items.empty?\n> + format = [ 'commit %H', 'author %an <%ae>', '', '%B' ].join('%n')\n> + File.popen(['git', 'show', '-z', '-s', '--format=format:' + format] + @items.keys) do |p|\n\nAh, you're using 'git show'. I thought cat-file --batch was\nespecially well-suited for this task. What do you need the\npretty-printing for?\n\n> + p.each(\"\\0\") do |data|\n> + next if data == \"\\0\" # bug in git show?\n\nWhat is the bug exactly? It displays two consecutive \\0 characters\nsometimes, when given a bunch of items to display?\n\n> + id, roles = Commit.parse(data)\n> + commit = @items[id]\n> + commit.roles = roles\n\n@items[id].roles = roles\n\n> + def each_person_role\n> + commit_roles = @items.values.map { |commit| commit.roles }.flatten(1)\n> + commit_roles.group_by { |person, role| person }.each do |person, commit_roles|\n> + commit_roles.group_by { |person, role| role }.each do |role, commit_roles|\n> + yield person, role, commit_roles.size\n\nUnnecessary work if you'd chosen a better way to store the person =>\nrole mapping in the first place.\n\n> + def get_blame(source, start, offset, from)\n> + return unless source\n> + File.popen(['git', 'blame', '--incremental', '-C',\n> + '-L', '%u,+%u' % [start, offset],\n> + '--since', $since, from + '^',\n> + '--', source]) do |p|\n\nWhile at it, why not blame -M -CCC?\n\n> + def from_patch(file)\n\nYou're not going to 'git mailsplit', like am does, first?\n\n> + when /^@@\\s-(\\d+),(\\d+)/\n> + get_blame(source, $1, $2, from)\n\nOkay, so this is where you get the arguments for the -L in blame.\n\n> +exit 1 if ARGV.size != 1\n\nNo usage?\n\n> +commits.each_person_role do |person, role, count|\n> + persons[person][role] = count\n\nOh dear. Don't you think you're over-engineering here? Just because\nyou can stuff anything into hashes/lists in Ruby, doesn't mean that\nyou should and kill efficiency.\n\n> +persons.each do |person, roles|\n> + roles = roles.map do |role, count|\n> + percent = count.to_f * 100 / commits.size\n\nInteresting. You also take into account the size of the commits when\ncalculating the final score.\n\n> + next if percent < $min_percent\n> + \"%s: %u%%\" % [role, percent]\n> + end.compact\n\nWhat are these nil elements you're filtering out with .compact?\n\nOverall, it looks like the whole thing is one giant first draft\nwritten in a big rush. For a relatively simple task, you've used a\nlot of complex data structures and complicated things beyond belief.\nI'm sorry, but this giant mess wasn't at all a pleasure to read. No\ndoubt that the idea is good though. I'm stopping here.\n"},{"id":"214836","messageId":"CAMP44s0sXk9xCXjY5bYsy2mrvkKrPPWqAVhZHChv+MHpNySPsg@mail.gmail.com","threadId":"33541","inReplyTo":"CALkWK0n+cWspBJtH7JFvsvoeHHuiWR0bGpYVU63e7O74XY=4sQ@mail.gmail.com","subject":"Re: [PATCH v2 1/8] Add new git-cc-cmd helper to contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-19T16:30:17Z","receivedAt":"2013-04-19T16:30:17Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Apr 19, 2013 at 8:26 AM, Ramkumar Ramachandra\n wrote:\n> Felipe Contreras wrote:\n>> The code finds the changes of a commit, runs 'git blame' for each chunk\n>> to see which other commits are relevant, and then reports the author and\n>> signers.\n>>\n>> Finally, it calculates what percentage of the total relevant commits\n>> each person was involved in, and show only the ones that pass the\n>> threshold.\n>\n> Um, this didn't really explain it to me. How about:\n>\n> This command takes a patch prepared by 'git format-patch' as an\n> argument, and runs 'git blame' on every hunk of the diff to determine\n> the commits that added/removed those lines. It then aggregates the\n> authors and signers of all the commits, and prints out these people in\n> descending order of the percentage of commits they were responsible\n> for. It omits people are below a certain threshold.\n\nFine by me.\n\n>> % git cc-cmd 0001-remote-hg-trivial-cleanups.patch\n>> Felipe Contreras (author: 100%)\n>> Jeff King (signer: 83%)\n>> Max Horn (signer: 16%)\n>> Junio C Hamano (signer: 16%)\n>\n> Won't my name appear as the first one on each of my patches? Will I\n> be CC'ed on every patch that I send out?\n\nNo. The patches being analyzed are ignored. Either way, you are the\ns-o-b, so send-email Cc's you by default.\n\n>> contrib/cc-cmd/git-cc-cmd | 140 ++++++++++++++++++++++++++++++++++++++++++++++\n>> 1 file changed, 140 insertions(+)\n>> create mode 100755 contrib/cc-cmd/git-cc-cmd\n>\n> No README?\n\nNo README. Half the stuff in contrib doesn't have one. And I don't see\nwhat stuff of value we could have there.\n\n>> diff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd\n>> new file mode 100755\n>> index 0000000..c7ecf79\n>> --- /dev/null\n>> +++ b/contrib/cc-cmd/git-cc-cmd\n>> @@ -0,0 +1,140 @@\n>> +#!/usr/bin/env ruby\n>\n> What is the minimum required version of Ruby, by the way?\n\nI don't know.\n\n> I see you\n> haven't used any fancy lazy enumerators\n\nI did, but they are implicit.\n\n>> +$since = '3-years-ago'\n>> +$min_percent = 5\n>\n> Do I get to configure these two?\n\nPatience. That's for another patch.\n\n> Also, the '$' prefix from your Perl\n> habit? It's not idiomatic in Ruby afaik.\n\nIt is for global variables.\n\n>> +class Commit\n>> +\n>> + attr_reader :id\n>> + attr_accessor :roles\n>> +\n>> + def initialize(id)\n>> + @id = id\n>> + @roles = []\n>\n> What are id, roles exactly? This isn't C where we have types, so\n> you're going to have to use some (rdoc-parseable) comments, if you\n> want me to be able to read the code without jumping around too much.\n\nThe id of the commit, of course (aka. SHA-1), and the roles people had\nin this commit. We are after all in the Commit class.\n\n>> + end\n>> +\n>> + def self.parse(data)\n>> + id = author = msg = nil\n>\n> This is not C. You don't need to initialize stuff, unless you're\n> going to start accessing it using a .append() or similar. In that\n> case, you need to initialize it as an empty list, so the compiler\n> knows what you're appending to.\n\nYou need to define them if you are going to modify them in a block,\nand access them after the block.\n\n>> + roles = {}\n>\n> Shouldn't this be @roles?\n\nNo, we are in a class method, we don't have instance variables; we\ndon't have an instance.\n\n> Didn't you initialize it as an empty list\n> in initialize()?\n\nNo, that's for the instance; we don't have an instance yet.\n\n> Why did you suddenly change your mind and make it a\n> hashtable now?\n\nIt's a completely different variable; it's local to the method.\n\n>> + data.each_line do |line|\n>> + if not msg\n>> + case line\n>> + when /^commit (.+)$/\n>> + id = $1\n>\n> Okay, I have no idea what you're parsing yet. There's some commit\n> line, so I know this is not a raw commit object, as the name of the\n> class Commit would've indicated.\n\nYeah, it's parsing a raw commit object.\n\n>> + when /^author ([^<>]+) <(\\S+)>$/\n>> + author = $1, $2\n>> + roles[author] = 'author'\n>\n> Huh? the key is a two-item list, and the value is a hard-coded\n> 'author' string? If it's meant to be a sematic value, use a symbol.\n\nYeah, that might make sense.\n\n> Also, why don't you just collect the authors and signers in two\n> separate lists, instead of doing this and burdening yourself with\n> accumulation later?\n\nGo ahead and try this change. How are you going to group persons by\nthe role they took in the relevant commits? It's much simpler to have\ngeneric code that is agnostic of the types of roles there are. Also,\nthis would make it simpler to add other roles.\n\n>> + when /^$/\n>> + msg = true\n>\n> I can't see msg being used later in the function, so I don't know what\n> you're doing.\n\nIt's used before.\n\n>> + roles = roles.map do |person, role|\n>> + address = \"%s <%s>\" % person\n>> + [person, role]\n>\n> What?! If you wanted address in the first place, why did you stuff a\n> two-member list into roles as the key?\n\nWhat difference does it make? 'person' could be an object, it could be anything.\n\n> Where is address being used?\n\nIn a later patch. This line creeped in.\n\n>> + def import\n>> + return if @items.empty?\n>> + format = [ 'commit %H', 'author %an <%ae>', '', '%B' ].join('%n')\n>> + File.popen(['git', 'show', '-z', '-s', '--format=format:' + format] + @items.keys) do |p|\n>\n> Ah, you're using 'git show'. I thought cat-file --batch was\n> especially well-suited for this task.\n\nPerhaps, but the code might end more complicated though.\n\n> What do you need the\n> pretty-printing for?\n\nTo generate a raw commit; the raw format is not really so.\n\n>> + p.each(\"\\0\") do |data|\n>> + next if data == \"\\0\" # bug in git show?\n>\n> What is the bug exactly? It displays two consecutive \\0 characters\n> sometimes, when given a bunch of items to display?\n\nApparently.\n\n>> + id, roles = Commit.parse(data)\n>> + commit = @items[id]\n>> + commit.roles = roles\n>\n> @items[id].roles = roles\n\nNoMethodError: undefined method `roles=' for nil:NilClass\n\n>> + def each_person_role\n>> + commit_roles = @items.values.map { |commit| commit.roles }.flatten(1)\n>> + commit_roles.group_by { |person, role| person }.each do |person, commit_roles|\n>> + commit_roles.group_by { |person, role| role }.each do |role, commit_roles|\n>> + yield person, role, commit_roles.size\n>\n> Unnecessary work if you'd chosen a better way to store the person =>\n> role mapping in the first place.\n\nNo. The contents of the person key are irrelevant here, they are\nmerely used as that: a key.\n\nIt's trivial to get the role a person took in a given commit; we are\nnot interested in that. We want the sum of all the author roles a\nperson had, and the signer roles, and any other rules that might be\nspecified in the future. *And* we want that count to be grouped first\nby person, and then by role, because that's precisely what we will\nshow the user.\n\nThere's no better way to represent the data of each commit than by a\nCommit object, and there's no better way to represent the roles each\nperson took in a certain commit, than a Hash. Which is exactly what we\nhave.\n\n>> + def get_blame(source, start, offset, from)\n>> + return unless source\n>> + File.popen(['git', 'blame', '--incremental', '-C',\n>> + '-L', '%u,+%u' % [start, offset],\n>> + '--since', $since, from + '^',\n>> + '--', source]) do |p|\n>\n> While at it, why not blame -M -CCC?\n\n-C implies -M, but the three times might make sense.\n\n>> + def from_patch(file)\n>\n> You're not going to 'git mailsplit', like am does, first?\n\nCould be added later.\n\n>> + when /^@@\\s-(\\d+),(\\d+)/\n>> + get_blame(source, $1, $2, from)\n>\n> Okay, so this is where you get the arguments for the -L in blame.\n>\n>> +exit 1 if ARGV.size != 1\n>\n> No usage?\n\nLater patch.\n\n>> +commits.each_person_role do |person, role, count|\n>> + persons[person][role] = count\n>\n> Oh dear. Don't you think you're over-engineering here? Just because\n> you can stuff anything into hashes/lists in Ruby, doesn't mean that\n> you should and kill efficiency.\n\nWrite the changes and show me the numbers. Ruby hashes are extremely\nefficient, it hardly matters for less than ten people you are going to\nCc, and any other code would be more complicated, if at all possible,\nI tried.\n\n>> +persons.each do |person, roles|\n>> + roles = roles.map do |role, count|\n>> + percent = count.to_f * 100 / commits.size\n>\n> Interesting. You also take into account the size of the commits when\n> calculating the final score.\n\nNo, it's the total number of commits, like any array.size.\n\n>> + next if percent < $min_percent\n>> + \"%s: %u%%\" % [role, percent]\n>> + end.compact\n>\n> What are these nil elements you're filtering out with .compact?\n\nThe ones that are below the threshold.\n\n> Overall, it looks like the whole thing is one giant first draft\n> written in a big rush.\n\nIt's not. The first draft was sent more than three years ago[1]. I\nhave literally been developing this for years, fixing related bugs in\ngit along the way [2], and shaping some features [3]. I sent the\nprevious version last year, which was basically the same design, and\nyou thought it was great and even asked why it wasn't merged[4].\n\nI have written and rewritten this code, simplified and refactored it.\nAnd I'm fairly certain this version is clean and tidy, and could\nhardly be made any simpler.\n\n> For a relatively simple task,\n\nThat is the problem right there; you don't understand what the code is\ndoing, so you think because the code is relatively small, you do\nunderstand it, and the task must be simple. I assure you, it's not.\n\nBut you have hands, go ahead and try to simplify it. If the task is so\nsimple, you can remove all the code you don't like, and see how it\nfares. Then you'll see.\n\n> I'm sorry, but this giant mess wasn't at all a pleasure to read.\n\nIt wasn't meant to be. It was meant to solve a problem in the simplest\nway possible.\n\nSurely the code can have some cosmetic changes to explain better what\nit's doing, but I doubt you find a way to substantially improve _how_\nit's doing it. Certainly none of the suggestions above did help in\nthat regard.\n\nCheers.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/130391\n[2] http://article.gmane.org/gmane.comp.version-control.git/189897\n[3] http://article.gmane.org/gmane.comp.version-control.git/210059\n[4] http://article.gmane.org/gmane.comp.version-control.git/209419\n\n-- \nFelipe Contreras\n"},{"id":"214846","messageId":"7vfvym30t8.fsf@alter.siamese.dyndns.org","threadId":"33541","inReplyTo":"1366348458-7706-2-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 1/8] Add new git-cc-cmd helper to contrib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-19T17:08:51Z","receivedAt":"2013-04-19T17:08:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n> The code finds the changes of a commit, runs 'git blame' for each chunk\n> to see which other commits are relevant, and then reports the author and\n> signers.\n\nIn general, I am not all that interested in adding anything new to\ncontrib/ as git.git has matured enough, but even if this will stay\noutside my tree, there are a few interesting things to note to help\nits eventual users.\n\n> + roles = roles.map do |person, role|\n> + address = \"%s <%s>\" % person\n> + [person, role]\n> + end\n\nIs address being used elsewhere, or is this a remnant from an\nearlier debugging or something?\n\n> + [id, roles]\n> + end\n> +\n> +end\n> ...\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\nThis may need to be tightened if you want to use this on a\nreal-world project (git.git itself does not count ;-); you may see\nsomething like:\n\n diff --git \"a/a\\\"b\" \"b/a\\\"b\"\n\n(I did an insane pathname 'a\"b' to get the above example, but a more\nrealistic is a character outside ASCII).\n\n> + when /^@@\\s-(\\d+),(\\d+)/\n> + get_blame(source, $1, $2, from)\n\nThis may want to be a bit more careful for a hunk that adds to an\nempty file, which will give you something like\n\n @@ -0,0 +1 @@\n @@ -0,0 +1,200 @@\n\nNobody sane would use -U0 when doing a format-patch, but if this\nwants to accomodate such a patch as well, it needs to ignore a hunk\nthat only adds new lines.\n"},{"id":"214851","messageId":"CAMP44s3YAq66MrOR5a4ydujKR5+ZNMVV4i=JzPCxLXC244b52g@mail.gmail.com","threadId":"33541","inReplyTo":"7vfvym30t8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 1/8] Add new git-cc-cmd helper to contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-19T17:35:09Z","receivedAt":"2013-04-19T17:35:09Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Apr 19, 2013 at 12:08 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>> The code finds the changes of a commit, runs 'git blame' for each chunk\n>> to see which other commits are relevant, and then reports the author and\n>> signers.\n>\n> In general, I am not all that interested in adding anything new to\n> contrib/ as git.git has matured enough, but even if this will stay\n> outside my tree, there are a few interesting things to note to help\n> its eventual users.\n\nWhy not add it to mainline git then? This tool, or a similar one,\nwould certainly be useful in the git arsenal.\n\n>> + roles = roles.map do |person, role|\n>> + address = \"%s <%s>\" % person\n>> + [person, role]\n>> + end\n>\n> Is address being used elsewhere, or is this a remnant from an\n> earlier debugging or something?\n\nIt's used later on; it creeped in.\n\n>> + [id, roles]\n>> + end\n>> +\n>> +end\n>> ...\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>\n> This may need to be tightened if you want to use this on a\n> real-world project (git.git itself does not count ;-); you may see\n> something like:\n>\n> diff --git \"a/a\\\"b\" \"b/a\\\"b\"\n>\n> (I did an insane pathname 'a\"b' to get the above example, but a more\n> realistic is a character outside ASCII).\n\nSuggestions on how to do that are welcome.\n\n>> + when /^@@\\s-(\\d+),(\\d+)/\n>> + get_blame(source, $1, $2, from)\n>\n> This may want to be a bit more careful for a hunk that adds to an\n> empty file, which will give you something like\n>\n> @@ -0,0 +1 @@\n> @@ -0,0 +1,200 @@\n\nSimple:\nreturn unless source and start and offset\n\n> Nobody sane would use -U0 when doing a format-patch, but if this\n> wants to accomodate such a patch as well, it needs to ignore a hunk\n> that only adds new lines.\n\nI'm not going to worry about it now.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"214856","messageId":"7vtxn21jvt.fsf@alter.siamese.dyndns.org","threadId":"33541","inReplyTo":"1366348458-7706-6-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH v2 5/8] contrib: cc-cmd: add option to parse from committish","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-19T17:59:50Z","receivedAt":"2013-04-19T17:59:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> For example master..feature-a.\n>\n> Signed-off-by: Felipe Contreras \n> ---\n> contrib/cc-cmd/git-cc-cmd | 36 ++++++++++++++++++++++++++++++++++--\n> 1 file changed, 34 insertions(+), 2 deletions(-)\n>\n> diff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd\n> index f13ed8f..462f22c 100755\n> --- a/contrib/cc-cmd/git-cc-cmd\n> +++ b/contrib/cc-cmd/git-cc-cmd\n> @@ -5,11 +5,13 @@ require 'optparse'\n> $since = '3-years-ago'\n> $min_percent = 5\n> $show_commits = false\n> +$files = []\n> +$rev_args = []\n> \n> begin\n> OptionParser.new do |opts|\n> opts.program_name = 'git cc-cmd'\n> - opts.banner = 'usage: git cc-cmd [options] '\n> + opts.banner = 'usage: git cc-cmd [options] '\n> \n> opts.on('-p', '--min-percent N', Integer, 'Minium percentage of role participation') do |v|\n> $min_percent = v\n> @@ -134,10 +136,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 --no-pager show -C --oneline] + [id]) do |p|\n\nWhen you know you are sending its output to a pipe, does --no-pager matter,\nor is there anything more subtle going on here?\n\nAn extra --no-pager does not hurt, but it just caught/distracted my\nattention while reading this patch.\n\n> + p.each do |e|\n> + case e\n> + when /^---\\s+(\\S+)/\n> + source = $1 != '/dev/null' ? $1[2..-1] : nil\n> + when /^@@\\s-(\\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"},{"id":"214861","messageId":"CAMP44s0ASLAaRMbGHsahHONzuGMC96rw5m72fM5EWpkYp==5uA@mail.gmail.com","threadId":"33541","inReplyTo":"7vtxn21jvt.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 5/8] contrib: cc-cmd: add option to parse from committish","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-19T18:29:10Z","receivedAt":"2013-04-19T18:29:10Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Apr 19, 2013 at 12:59 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> For example master..feature-a.\n>>\n>> Signed-off-by: Felipe Contreras \n>> ---\n>> contrib/cc-cmd/git-cc-cmd | 36 ++++++++++++++++++++++++++++++++++--\n>> 1 file changed, 34 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd\n>> index f13ed8f..462f22c 100755\n>> --- a/contrib/cc-cmd/git-cc-cmd\n>> +++ b/contrib/cc-cmd/git-cc-cmd\n>> @@ -5,11 +5,13 @@ require 'optparse'\n>> $since = '3-years-ago'\n>> $min_percent = 5\n>> $show_commits = false\n>> +$files = []\n>> +$rev_args = []\n>>\n>> begin\n>> OptionParser.new do |opts|\n>> opts.program_name = 'git cc-cmd'\n>> - opts.banner = 'usage: git cc-cmd [options] '\n>> + opts.banner = 'usage: git cc-cmd [options] '\n>>\n>> opts.on('-p', '--min-percent N', Integer, 'Minium percentage of role participation') do |v|\n>> $min_percent = v\n>> @@ -134,10 +136,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 --no-pager show -C --oneline] + [id]) do |p|\n>\n> When you know you are sending its output to a pipe, does --no-pager matter,\n> or is there anything more subtle going on here?\n>\n> An extra --no-pager does not hurt, but it just caught/distracted my\n> attention while reading this patch.\n\nIt probably doesn't matter, I might have been using something else\nwhen I copied that code.\n\n-- \nFelipe Contreras\n"},{"id":"214864","messageId":"7v8v4e1fyz.fsf@alter.siamese.dyndns.org","threadId":"33541","inReplyTo":"CAMP44s3YAq66MrOR5a4ydujKR5+ZNMVV4i=JzPCxLXC244b52g@mail.gmail.com","subject":"Re: [PATCH v2 1/8] Add new git-cc-cmd helper to contrib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-19T19:24:20Z","receivedAt":"2013-04-19T19:24:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> On Fri, Apr 19, 2013 at 12:08 PM, Junio C Hamano wrote:\n>> Felipe Contreras writes:\n>>> The code finds the changes of a commit, runs 'git blame' for each chunk\n>>> to see which other commits are relevant, and then reports the author and\n>>> signers.\n>>\n>> In general, I am not all that interested in adding anything new to\n>> contrib/ as git.git has matured enough, but even if this will stay\n>> outside my tree, there are a few interesting things to note to help\n>> its eventual users.\n>\n> Why not add it to mainline git then? This tool, or a similar one,\n> would certainly be useful in the git arsenal.\n\nAs to this particular \"feature\" (the goal it tries to achieve, not\nnecessarily the implementation), that actually was the first thing\nthat came to my mind. It helps the \"develop, review locally,\nformat-patch, decide whom to ask reviews and then send it out\"\nworkflow in general to have a tool that tells you who are the people\ninvolved in the code you are touching.\n\nIf this were _only_ to be used within send-email (i.e. replacing the\n\"then send it out\" above with \"then use send-email\" to limit the\nusecase), \"git cc-cmd\" would be a reasonable name. But if that is\nthe intended use case, it would even be more reasonable to make this\nlogic part of send-email and trigger it with --auto-cc-reviewers\noption or something.\n\nBut I think it can be useful outside the context of send-email as\nwell, and having one independent tool that does one single job well\nis a better design. Perhaps it is better to name it less specific\nto send-email's cc-cmd option. \"git people\"? \"git whom\"? \"git\nreviewers\"? I dunno, but along those lines.\n\nIt is OK for a design demonstration prototype to be written in any\nlanguage others (who can comment on the design) can read, but the\nversion to be a first-class citizen needs to be written in one of\nthe languages such as C, POSIX shell, or Perl to avoid adding extra\ndependencies to the users.\n"},{"id":"214876","messageId":"CAMP44s2gA0JbfxA1UQW_pnizGBpmbQem3Qg0FpWP_Wi6eYwVjw@mail.gmail.com","threadId":"33541","inReplyTo":"7v8v4e1fyz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 1/8] Add new git-cc-cmd helper to contrib","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-04-19T19:35:57Z","receivedAt":"2013-04-19T19:35:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Fri, Apr 19, 2013 at 2:24 PM, Junio C Hamano wrote:\n> Felipe Contreras writes:\n>\n>> On Fri, Apr 19, 2013 at 12:08 PM, Junio C Hamano wrote:\n>>> Felipe Contreras writes:\n>>>> The code finds the changes of a commit, runs 'git blame' for each chunk\n>>>> to see which other commits are relevant, and then reports the author and\n>>>> signers.\n>>>\n>>> In general, I am not all that interested in adding anything new to\n>>> contrib/ as git.git has matured enough, but even if this will stay\n>>> outside my tree, there are a few interesting things to note to help\n>>> its eventual users.\n>>\n>> Why not add it to mainline git then? This tool, or a similar one,\n>> would certainly be useful in the git arsenal.\n>\n> As to this particular \"feature\" (the goal it tries to achieve, not\n> necessarily the implementation), that actually was the first thing\n> that came to my mind. It helps the \"develop, review locally,\n> format-patch, decide whom to ask reviews and then send it out\"\n> workflow in general to have a tool that tells you who are the people\n> involved in the code you are touching.\n>\n> If this were _only_ to be used within send-email (i.e. replacing the\n> \"then send it out\" above with \"then use send-email\" to limit the\n> usecase), \"git cc-cmd\" would be a reasonable name. But if that is\n> the intended use case, it would even be more reasonable to make this\n> logic part of send-email and trigger it with --auto-cc-reviewers\n> option or something.\n\nYeap, but I wouldn't want to be the one that implements that in perl.\n\n> But I think it can be useful outside the context of send-email as\n> well, and having one independent tool that does one single job well\n> is a better design. Perhaps it is better to name it less specific\n> to send-email's cc-cmd option. \"git people\"? \"git whom\"? \"git\n> reviewers\"? I dunno, but along those lines.\n\n'git relevant'? 'git related'? It's not only people, also commits.\n\n> It is OK for a design demonstration prototype to be written in any\n> language others (who can comment on the design) can read, but the\n> version to be a first-class citizen needs to be written in one of\n> the languages such as C, POSIX shell, or Perl to avoid adding extra\n> dependencies to the users.\n\nThat is going to be though.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"214882","messageId":"7vr4i6z448.fsf@alter.siamese.dyndns.org","threadId":"33541","inReplyTo":"CAMP44s2gA0JbfxA1UQW_pnizGBpmbQem3Qg0FpWP_Wi6eYwVjw@mail.gmail.com","subject":"Re: [PATCH v2 1/8] Add new git-cc-cmd helper to contrib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-19T19:56:23Z","receivedAt":"2013-04-19T19:56:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n>> If this were _only_ to be used within send-email (i.e. replacing the\n>> \"then send it out\" above with \"then use send-email\" to limit the\n>> usecase), \"git cc-cmd\" would be a reasonable name. But if that is\n>> the intended use case, it would even be more reasonable to make this\n>> logic part of send-email and trigger it with --auto-cc-reviewers\n>> option or something.\n>\n> Yeap, but I wouldn't want to be the one that implements that in perl.\n\nThat is OK. None of this has to be done by you.\n\nAnd we seem to be in agreement that the feature deserves to be its\nown command, so it does not have to be in Perl, either.\n\n>> But I think it can be useful outside the context of send-email as\n>> well, and having one independent tool that does one single job well\n>> is a better design. Perhaps it is better to name it less specific\n>> to send-email's cc-cmd option. \"git people\"? \"git whom\"? \"git\n>> reviewers\"? I dunno, but along those lines.\n>\n> 'git relevant'? 'git related'? It's not only people, also commits.\n\nLet's let it simmer on the list for a few days so that other people\ncan come up with a better name.\n"},{"id":"214885","messageId":"5171A387.808@kdbg.org","threadId":"33541","inReplyTo":"7v8v4e1fyz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 1/8] Add new git-cc-cmd helper to contrib","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2013-04-19T20:05:27Z","receivedAt":"2013-04-19T20:05:27Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 19.04.2013 21:24, schrieb Junio C Hamano:\n> Felipe Contreras writes:\n> \n>> On Fri, Apr 19, 2013 at 12:08 PM, Junio C Hamano wrote:\n>>> Felipe Contreras writes:\n>>>> The code finds the changes of a commit, runs 'git blame' for each chunk\n>>>> to see which other commits are relevant, and then reports the author and\n>>>> signers.\n> \n> But I think it can be useful outside the context of send-email as\n> well, and having one independent tool that does one single job well\n> is a better design. Perhaps it is better to name it less specific\n> to send-email's cc-cmd option. \"git people\"? \"git whom\"? \"git\n> reviewers\"? I dunno, but along those lines.\n\nWould it make sense to integrate this in git shortlog, which already\ndoes something similar?\n\n-- Hannes\n"},{"id":"214886","messageId":"7vip3iz3jn.fsf@alter.siamese.dyndns.org","threadId":"33541","inReplyTo":"CAMP44s3YAq66MrOR5a4ydujKR5+ZNMVV4i=JzPCxLXC244b52g@mail.gmail.com","subject":"Re: [PATCH v2 1/8] Add new git-cc-cmd helper to contrib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-19T20:08:44Z","receivedAt":"2013-04-19T20:08:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\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>>\n>> This may need to be tightened if you want to use this on a\n>> real-world project (git.git itself does not count ;-); you may see\n>> something like:\n>>\n>> diff --git \"a/a\\\"b\" \"b/a\\\"b\"\n>>\n>> (I did an insane pathname 'a\"b' to get the above example, but a more\n>> realistic is a character outside ASCII).\n>\n> Suggestions on how to do that are welcome.\n\nCheck gitweb and find \"unquote\".\n"},{"id":"214940","messageId":"7vip3hx88x.fsf@alter.siamese.dyndns.org","threadId":"33541","inReplyTo":"5171A387.808@kdbg.org","subject":"Re: [PATCH v2 1/8] Add new git-cc-cmd helper to contrib","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-20T20:22:22Z","receivedAt":"2013-04-20T20:22:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n>> But I think it can be useful outside the context of send-email as\n>> well, and having one independent tool that does one single job well\n>> is a better design. Perhaps it is better to name it less specific\n>> to send-email's cc-cmd option. \"git people\"? \"git whom\"? \"git\n>> reviewers\"? I dunno, but along those lines.\n>\n> Would it make sense to integrate this in git shortlog, which already\n> does something similar?\n\nConceptually, yes, but the end result will be much larger in scope.\nI am not sure if \"shortlog\" is still a good label for it.\n\n\"shortlog\", when it internally runs \"log\" [*1*], is still about the\ncommits within the range given to the command; \"shortlog A..B\" talks\nonly about commits within A..B range. This new thing is about what\nhappened to the part of the code A..B touches in the past\n(i.e. before A happened), which feels a bit different.\n\n\n[Footnote]\n\n*1* It can be used as a filter to \"git log\" output, which is a bit\n different animal, but it still is about shortening that incoming\n log, not about independently digging the history using the input\n as a starting point.\n"},{"id":"215062","messageId":"2120794796.1800558.1366615830636.JavaMail.root@openwide.fr","threadId":"33541","inReplyTo":"7vip3hx88x.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 1/8] Add new git-cc-cmd helper to contrib","fromName":"Jeremy Rosen","fromEmail":"jeremy.rosen@openwide.fr","sentAt":"2013-04-22T07:30:30Z","receivedAt":"2013-04-22T07:30:30Z","isPatch":true,"sender":{"key":"jeremy.rosen@openwide.fr","avatar":null},"body":"> >\n> > Would it make sense to integrate this in git shortlog, which\n> > already\n> > does something similar?\n> \n> Conceptually, yes, but the end result will be much larger in scope.\n> I am not sure if \"shortlog\" is still a good label for it.\n> \n\nsince we are throwing ideas around...\n\nThe first place where I would logically look for such a feature would be\nin git-blame --cc-list or something like that.\n\ngit-blame seems to me as a logical place for all \"look at history and give\nme a list of names\" type commands\n"}]}