Re: [PATCH v2 5/8] contrib: cc-cmd: add option to parse from committish

2013-04-19 Thread Felipe Contreras
On Fri, Apr 19, 2013 at 12:59 PM, Junio C Hamano  wrote:
> Felipe Contreras  writes:
>
>> For example master..feature-a.
>>
>> Signed-off-by: Felipe Contreras 
>> ---
>>  contrib/cc-cmd/git-cc-cmd | 36 ++--
>>  1 file changed, 34 insertions(+), 2 deletions(-)
>>
>> diff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd
>> index f13ed8f..462f22c 100755
>> --- a/contrib/cc-cmd/git-cc-cmd
>> +++ b/contrib/cc-cmd/git-cc-cmd
>> @@ -5,11 +5,13 @@ require 'optparse'
>>  $since = '3-years-ago'
>>  $min_percent = 5
>>  $show_commits = false
>> +$files = []
>> +$rev_args = []
>>
>>  begin
>>OptionParser.new do |opts|
>>  opts.program_name = 'git cc-cmd'
>> -opts.banner = 'usage: git cc-cmd [options] '
>> +opts.banner = 'usage: git cc-cmd [options] '
>>
>>  opts.on('-p', '--min-percent N', Integer, 'Minium percentage of role 
>> participation') do |v|
>>$min_percent = v
>> @@ -134,10 +136,40 @@ class Commits
>>  end
>>end
>>
>> +  def from_rev_args(args)
>> +return if args.empty?
>> +source = nil
>> +File.popen(%w[git rev-list --reverse] + args) do |p|
>> +  p.each do |e|
>> +id = e.chomp
>> +@main_commits[id] = true
>> +File.popen(%w[git --no-pager show -C --oneline] + [id]) do |p|
>
> When you know you are sending its output to a pipe, does --no-pager matter,
> or is there anything more subtle going on here?
>
> An extra --no-pager does not hurt, but it just caught/distracted my
> attention while reading this patch.

It probably doesn't matter, I might have been using something else
when I copied that code.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 5/8] contrib: cc-cmd: add option to parse from committish

2013-04-19 Thread Junio C Hamano
Felipe Contreras  writes:

> For example master..feature-a.
>
> Signed-off-by: Felipe Contreras 
> ---
>  contrib/cc-cmd/git-cc-cmd | 36 ++--
>  1 file changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/cc-cmd/git-cc-cmd b/contrib/cc-cmd/git-cc-cmd
> index f13ed8f..462f22c 100755
> --- a/contrib/cc-cmd/git-cc-cmd
> +++ b/contrib/cc-cmd/git-cc-cmd
> @@ -5,11 +5,13 @@ require 'optparse'
>  $since = '3-years-ago'
>  $min_percent = 5
>  $show_commits = false
> +$files = []
> +$rev_args = []
>  
>  begin
>OptionParser.new do |opts|
>  opts.program_name = 'git cc-cmd'
> -opts.banner = 'usage: git cc-cmd [options] '
> +opts.banner = 'usage: git cc-cmd [options] '
>  
>  opts.on('-p', '--min-percent N', Integer, 'Minium percentage of role 
> participation') do |v|
>$min_percent = v
> @@ -134,10 +136,40 @@ class Commits
>  end
>end
>  
> +  def from_rev_args(args)
> +return if args.empty?
> +source = nil
> +File.popen(%w[git rev-list --reverse] + args) do |p|
> +  p.each do |e|
> +id = e.chomp
> +@main_commits[id] = true
> +File.popen(%w[git --no-pager show -C --oneline] + [id]) do |p|

When you know you are sending its output to a pipe, does --no-pager matter,
or is there anything more subtle going on here?

An extra --no-pager does not hurt, but it just caught/distracted my
attention while reading this patch.

> +  p.each do |e|
> +case e
> +when /^---\s+(\S+)/
> +  source = $1 != '/dev/null' ? $1[2..-1] : nil
> +when /^@@\s-(\d+),(\d+)/
> +  get_blame(source, $1, $2, id)
> +end
> +  end
> +end
> +  end
> +end
> +  end
> +
> +end
> +
> +ARGV.each do |e|
> +  if File.exists?(e)
> +$files << e
> +  else
> +$rev_args << e
> +  end
>  end
>  
>  commits = Commits.new
> -commits.from_patches(ARGV)
> +commits.from_patches($files)
> +commits.from_rev_args($rev_args)
>  commits.import
>  
>  if $show_commits
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html