Re: [RFCv4 2/3] gitweb: add patches view

[Jakub Narebski <jnareb@gmail.com>]

On Sat, 6 Dec 2008 16:02, Giuseppe Bilotta wrote:

> The only difference between patch and patches view is in the treatement
> of single commits: the former only displays a single patch, whereas the
> latter displays a patchset leading to the specified commit.

I like that fact that we have "patches" action which intent is to
show series of patches, and "patch" action which intent is to show
single patch. I'm just not sure if "patch" view should not simply
ignore $hash_parent...

Signoff?

> ---
>  gitweb/gitweb.perl |   20 ++++++++++++++++++--
>  1 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 71d5af4..dfc7128 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -525,6 +525,7 @@ our %actions = (
>  	"history" => \&git_history,
>  	"log" => \&git_log,
>  	"patch" => \&git_patch,
> +	"patches" => \&git_patches,
>  	"rss" => \&git_rss,
>  	"atom" => \&git_atom,
>  	"search" => \&git_search,
> @@ -5408,6 +5409,9 @@ sub git_blobdiff_plain {
>  
>  sub git_commitdiff {
>  	my $format = shift || 'html';
> +	# for patch view: should we limit ourselves to a single patch
> +	# if only a single commit is passed?
> +	my $single_patch = shift && 1;

What does this "shift && 1" does? Equivalent of "!!shift"?
Is it really needed?

Perhaps it would be better to use %opts trick, like for some other
gitweb subroutines (-single=>1, or -single_patch=>1, or -nmax=>1)?
Or perhaps not...

>  
>  	my $patch_max;
>  	if ($format eq 'patch') {
> @@ -5524,7 +5528,15 @@ sub git_commitdiff {
>  			}
>  			push @commit_spec, '-n', "$hash_parent..$hash";
>  		} else {
> -			push @commit_spec, '-1', '--root', $hash;
> +			if ($single_patch) {
> +				push @commit_spec, '-1';
> +			} else {
> +				if ($patch_max > 0) {
> +					push @commit_spec, "-$patch_max";
> +				}
> +				push @commit_spec, "-n";
> +			}
> +			push @commit_spec, '--root', $hash;

Nice.

>  		}
>  		open $fd, "-|", git_cmd(), "format-patch", '--encoding=utf8',
>  			'--stdout', @commit_spec
> @@ -5620,7 +5632,11 @@ sub git_commitdiff_plain {
>  
>  # format-patch-style patches
>  sub git_patch {
> -	git_commitdiff('patch');
> +	git_commitdiff('patch', 1);
> +}
> +
> +sub git_patches {
> +	git_commitdiff('patch', 0);

I quite like it.

>  }
>  
>  sub git_history {
> -- 
> 1.5.6.5
> 
> 

-- 
Jakub Narebski
Poland