Subject: Re: [PATCH 14/18] gitweb: add print_transient_header() function for central header printing

From: Jakub Narebski <[email-masked]>
Date: Thu, 09 Dec 2010 16:36:09 -0800 (PST)

"John 'Warthog9' Hawley" <[email-masked]> writes:

> There are a few things I would like to reuse the transient header
> information I'm using, currently this is only the 'Generating...'
> page, but there is at least one additional warning page I would
> like to use this on.
> 
> Signed-off-by: John 'Warthog9' Hawley <[email-masked]>
> ---
>  gitweb/lib/cache.pl |   47 ++++++++++++++++++++++++++---------------------
>  1 files changed, 26 insertions(+), 21 deletions(-)
> 
> diff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl
> index 63dbe9e..723ae9b 100644
> --- a/gitweb/lib/cache.pl
> +++ b/gitweb/lib/cache.pl
> @@ -94,6 +94,31 @@ sub cache_fetch {
>  	#$actions{$action}->();
>  }
>  
> +sub print_transient_header {
> +	print $::cgi->header(

Why you use $::cgi->header() instead of equivalent $cgi->header()?
Note that $::cgi->header() is $main::cgi->header(), and is not
CGI::header().

> +				-type=>'text/html',
> +				-charset => 'utf-8',
> +				-status=> 200,
> +				-expires => 'now',
> +				# HTTP/1.0
> +				-Pragma => 'no-cache',
> +				# HTTP/1.1
> +				-Cache_Control => join(
> +							', ',
> +							qw(
> +								private
> +								no-cache
> +								no-store
> +								must-revalidate
> +								max-age=0
> +								pre-check=0
> +								post-check=0
> +							)
> +						)
> +				);
> +	return;
> +}

Why not use

	our %no_cache = (
		# HTTP/1.0
		-Pragma => 'no-cache',
		# HTTP/1.1
		-Cache_Control => join(', ', qw(private no-cache no-store must-revalidate
		                                max-age=0 pre-check=0 post-check=0)),
	);

(or something like that).  This way you can reuse it even if content
type is different (e.g. 'text/plain').

But that is just a proposal.

> +
>  sub isBinaryAction {
>  	my ($action) = @_;
>  
> @@ -292,27 +317,7 @@ sub cacheWaitForUpdate {
>  
>  	$| = 1;
>  
> -	print $::cgi->header(
> -				-type=>'text/html',
> -				-charset => 'utf-8',
> -				-status=> 200,
> -				-expires => 'now',
> -				# HTTP/1.0
> -				-Pragma => 'no-cache',
> -				# HTTP/1.1
> -				-Cache_Control => join(
> -							', ',
> -							qw(
> -								private
> -								no-cache
> -								no-store
> -								must-revalidate
> -								max-age=0
> -								pre-check=0
> -								post-check=0
> -							)
> -						)
> -				);
> +	print_transient_header();
>  
>  	print <<EOF;
>  <!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www/w3.porg/TR/html4/strict.dtd">
> -- 
> 1.7.2.3
> 

-- 
Jakub Narebski
Poland
ShadeHawk on #git