Re: [PATCH 10/18] gitweb: Adding isBinaryAction() and isFeedAction() to determine the action type

[Jakub Narebski <jnareb@gmail.com>]

"John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:

> This is fairly self explanitory, these are here just to centralize the checking
> for these types of actions, as special things need to be done with regards to
> them inside the caching engine.
> 
> isBinaryAction() returns true if the action deals with creating binary files
> (this needing :raw output)

Why do you need special case binary / :raw output?  It is not really
necessary if it is done in right way, as shown in my rewrite.

> isFeedAction() returns true if the action deals with a news feed of some sort,
> basically used to bypass the 'Generating...' message should it be a news reader
> as those will explode badly on that page.

Why blacklisting 'feed', instead of whitelisting HTML-output?


BTW., please don't use mixedCase names, but underline_separated.

> 
> Signed-off-by: John 'Warthog9' Hawley <warthog9@eaglescrag.net>
> ---
>  gitweb/lib/cache.pl |   69 ++++++++++++++++++++++++++-------------------------
>  1 files changed, 35 insertions(+), 34 deletions(-)
> 
> diff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl
> index a8ee99e..d55b572 100644
> --- a/gitweb/lib/cache.pl
> +++ b/gitweb/lib/cache.pl
> @@ -88,6 +88,34 @@ sub cache_fetch {
>  	#$actions{$action}->();
>  }
>  
> +sub isBinaryAction {
> +	my ($action) = @_;
> +
> +	if(
> +		$action eq "snapshot"
> +		||
> +		$action eq "blob_plain"
> +	){
> +		return 1;	# True
> +	}
> +
> +	return 0;		# False
> +}
> +
> +sub isFeedAction {
> +	if(
> +		$action eq "atom"
> +		||
> +		$action eq "rss"
> +		||
> +		$action eq "opml"
> +	){
> +		return 1;	# True
> +	}
> +
> +	return 0;		# False
> +}

Compare to:

+our %actions_info = ();
+sub evaluate_actions_info {
+       our %actions_info;
+       our (%actions);
+
+       # unless explicitely stated otherwise, default output format is html
+       foreach my $action (keys %actions) {
+               $actions_info{$action}{'output_format'} = 'html';
+       }
+       # list all exceptions; undef means variable (no definite format)
+       map { $actions_info{$_}{'output_format'} = 'text' }
+               qw(commitdiff_plain patch patches project_index blame_data);
+       map { $actions_info{$_}{'output_format'} = 'xml' }
+               qw(rss atom opml); # there are different types (document formats) of XML
+       map { $actions_info{$_}{'output_format'} = undef }
+               qw(blob_plain object);
+       $actions_info{'snapshot'}{'output_format'} = 'binary';
+}

Instead of 'xml' you can use 'feed'.

Then e.g.:

+sub action_outputs_html {
+       my $action = shift;
+       return $actions_info{$action}{'output_format'} eq 'html';
+}


See 
  "gitweb: Introduce %actions_info, gathering information about actions"
  "gitweb: Show appropriate "Generating..." page when regenerating cache"
  http://repo.or.cz/w/git/jnareb-git.git/shortlog/refs/heads/origin..refs/heads/gitweb/cache-kernel-v6

-- 
Jakub Narebski
Poland
ShadeHawk on #git