"John 'Warthog9' Hawley" <warthog9@xxxxxxxxxxxxxx> 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.
>
> This makes use of print_transient_header() which is also used in the
> 'Generating...' page. Currently the only warning it throws is about
> the cache needing to be created. If that fails it's a fatal error
> and we call die_error()
Why do you feel the need to single out this case giving it warning,
and single out this warning by showing warning page?
Nevertheless show_warning() _might_ be a good idea.
>
> Signed-off-by: John 'Warthog9' Hawley <warthog9@xxxxxxxxxxxxxx>
> ---
> gitweb/lib/cache.pl | 36 +++++++++++++++++++++++++++++++++---
> 1 files changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/gitweb/lib/cache.pl b/gitweb/lib/cache.pl
> index 723ae9b..28e4240 100644
> --- a/gitweb/lib/cache.pl
> +++ b/gitweb/lib/cache.pl
> @@ -25,9 +25,13 @@ sub cache_fetch {
> my $cacheTime = 0;
>
> if(! -d $cachedir){
> - print "*** Warning ***: Caching enabled but cache directory does not exsist. ($cachedir)\n";
> - mkdir ("cache", 0755) || die "Cannot create cache dir - you will need to manually create";
> - print "Cache directory created successfully\n";
> + mkdir ("cache", 0755) || die_error(500, "Internal Server Error", "Cannot create cache dir () - you will need to manually create");
> + show_warning(
> + "<p>".
> + "<strong>*** Warning ***:</strong> Caching enabled but cache directory did not exsist. ($cachedir)<br/>/\n".
Minor nit: s/exsist/exist/
Don't you need to use esc_path() on $cachedir,
using either
...did not exist. (".esc_path($cachedir).")<br/>\n";
or using this trick
...did not exist. (@{[esc_path($cachedir)]})<br/>\n";
> + "Cache directory created successfully\n".
> + "<p>"
> + );
> }
>
> $full_url = "$my_url?". $ENV{'QUERY_STRING'};
> @@ -119,6 +123,32 @@ sub print_transient_header {
> return;
> }
>
> +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.
> +
> + my $warning = esc_html(shift) || "Unknown Warning";
> +
> + print_transient_header();
> +
> + print <<EOF;
> +<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www/w3.porg/TR/html4/strict.dtd";>
> +<!-- git web w/caching interface version $version, (C) 2006-2010, John 'Warthog9' Hawley <warthog9\@kernel.org> -->
> +<!-- git core binaries version $git_version -->
> +<head>
> +<meta http-equiv="content-type" content="$content_type; charset=utf-8"/>
$content_type is not defined here.
> +<meta name="generator" content="gitweb/$version git/$git_version"/>
> +<meta name="robots" content="index, nofollow"/>
It is "noindex, nofollow", isn't it?
> +<meta http-equiv="refresh" content="10"/>
Why 10 seconds?
> +<title>$title</title>
$title is not defined here.
> +</head>
> +<body>
> +$warning
> +</body>
> +</html>
> +EOF
> + exit(0);
"exit(0)" and not "goto DONE_GITWEB", or "goto DONE_REQUEST"?
> +}
> +
> sub isBinaryAction {
> my ($action) = @_;
Didn't you ran gitweb tests?
--
Jakub Narebski
Poland
ShadeHawk on #git
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html