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

From: "J.H." <[email-masked]>
Date: Thu, 09 Dec 2010 19:39:47 -0800

>> 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.

Because that's not how my caching engine does it, and the reason for
that is I am mimicking how the rest of gitweb does it.

I attempted at one point to do as you were suggesting, and it became too
cumbersome.  I eventually broke out the 'binary' packages into a special
case (thus mimicking how gitweb is already doing things), which also
gives me the advantage of being able to checksum the resulting binary
out of band, as well as being able to more trivially calculate the file
size being sent.

>> 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?

There are a limited number of feed types and their ilk (standard xml
formatted feed and atom), there are lots of html-output like things.
Easier to default and have things work, generally, than to have things
not work the way you would expect.

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

fixed in v9

- John 'Warthog9' Hawley