Re: [PATCH 15/18] gitweb: Add show_warning() to display an immediate warning, with refresh

From: J.H. <warthog9@eaglescrag.net>
Date: Dec 10, 2010, 07:38 UTC
Message-ID: <4D01D902.1030102@eaglescrag.net>
In-Reply-To: <m3zksezbkm.fsf@localhost.localdomain>

On 12/09/2010 05:01 PM, Jakub Narebski wrote:

Show 10 quoted lines

> "John 'Warthog9' Hawley" <warthog9@eaglescrag.net> writes:
> 
>> die_error() is an immediate and abrupt action.  show_warning() more or less
>> functions identically, except that the page generated doesn't use the
>> gitweb header or footer (in case they are broken) and has an auto-refresh
>> (10 seconds) built into it.
> 
> Why not use gitweb header/footer?  If they are broken, it should be
> caught in git development.  If we don't se them, the show_warning()
> output would look out of place.

The only other 'transient' style page, the 'Generating...' page doesn't use it, and I felt that since this was also transient, and only (likely) to be seen once it wasn't worth the header & footer.

That said I've added it back in, in v9.

Show 7 quoted lines

>> +sub show_warning {
>> +	$| = 1;
> 
>   +	local $| = 1;
> 
> $| is global variable, and otherwise you would turn autoflush for all
> code, which would matter e.g. for FastCGI.

Since the execution exits immediately after, wouldn't FastCGI reset at that point, since execution of that thread has stopped? Or does FastCGI retain everything as is across subsequent executions of a process?

>> +<meta http-equiv="refresh" content="10"/>
> 
> Why 10 seconds?

Long enough to see the error, but not too long to be a nuisance. Mainly just there to warn the admin that it did something automatic they may not have been expecting.

Show 9 quoted lines

>> +</head>
>> +<body>
>> +$warning
>> +</body>
>> +</html>
>> +EOF
>> +	exit(0);
> 
> "exit(0)" and not "goto DONE_GITWEB", or "goto DONE_REQUEST"?

DONE_REQUEST doesn't actually exist as a label, the exit was used partially for my lack of love for goto's, but mostly out of not realizing what that was calling back to (mainly for the excitement of things like PSGI and their ilk)

I will change that that, but considering there are other locations where I do explicit exit's and those are actually inherent to the way the caching engine currently works, I might need to go take a look at what's going on with respect to multi-threaded items inside of PSGI and their like. It's possible the caching engine doesn't actually work on those...

Show 6 quoted lines

>> +}
>> +
>>  sub isBinaryAction {
>>  	my ($action) = @_;
> 
> Didn't you ran gitweb tests?

I did, they passed for me - for whatever reason my cache dir wasn't cleaned up, and stayed resident once it was created.

- John 'Warthog9' Hawley