From: Jakub Narebski <jnareb@gmail•com>
To: "John 'Warthog9' Hawley" <warthog9@eaglescrag•net>
Cc: git@vger•kernel.org, Jakub Narebski <jnareb@gmail•com>
Subject: Re: [PATCH 15/18] gitweb: Add show_warning() to display an immediate warning, with refresh
Date: Thu, 09 Dec 2010 17:01:05 -0800 (PST) [thread overview]
Message-ID: <m3zksezbkm.fsf@localhost.localdomain> (raw)
In-Reply-To: <1291931844-28454-16-git-send-email-warthog9@eaglescrag.net>
"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.
>
> 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@eaglescrag•net>
> ---
> 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
next prev parent reply other threads:[~2010-12-10 1:01 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-09 21:57 [PATCH 00/18] Gitweb caching v8 John 'Warthog9' Hawley
2010-12-09 21:57 ` [PATCH 01/18] gitweb: Prepare for splitting gitweb John 'Warthog9' Hawley
2010-12-09 23:30 ` Jakub Narebski
2010-12-09 21:57 ` [PATCH 02/18] gitweb: add output buffering and associated functions John 'Warthog9' Hawley
2010-12-09 21:57 ` [PATCH 03/18] gitweb: File based caching layer (from git.kernel.org) John 'Warthog9' Hawley
2010-12-09 21:57 ` [PATCH 04/18] gitweb: Minimal testing of gitweb caching John 'Warthog9' Hawley
2010-12-09 21:57 ` [PATCH 05/18] gitweb: Regression fix concerning binary output of files John 'Warthog9' Hawley
2010-12-09 23:33 ` Jakub Narebski
2010-12-09 21:57 ` [PATCH 06/18] gitweb: Add more explicit means of disabling 'Generating...' page John 'Warthog9' Hawley
2010-12-09 21:57 ` [PATCH 07/18] gitweb: Revert back to $cache_enable vs. $caching_enabled John 'Warthog9' Hawley
2010-12-09 23:38 ` Jakub Narebski
2010-12-10 2:38 ` J.H.
2010-12-10 13:48 ` Jakub Narebski
2010-12-09 21:57 ` [PATCH 08/18] gitweb: Change is_cacheable() to return true always John 'Warthog9' Hawley
2010-12-09 23:46 ` Jakub Narebski
2010-12-09 21:57 ` [PATCH 09/18] gitweb: Revert reset_output() back to original code John 'Warthog9' Hawley
2010-12-09 23:58 ` Jakub Narebski
2010-12-10 2:43 ` J.H.
2010-12-09 21:57 ` [PATCH 10/18] gitweb: Adding isBinaryAction() and isFeedAction() to determine the action type John 'Warthog9' Hawley
2010-12-10 0:06 ` Jakub Narebski
2010-12-10 3:39 ` J.H.
2010-12-10 12:10 ` Jakub Narebski
2010-12-10 12:25 ` Jakub Narebski
2010-12-09 21:57 ` [PATCH 11/18] gitweb: add isDumbClient() check John 'Warthog9' Hawley
2010-12-10 0:12 ` Jakub Narebski
2010-12-10 4:00 ` J.H.
2010-12-11 0:07 ` Junio C Hamano
2010-12-11 0:15 ` Jakub Narebski
2010-12-11 1:15 ` J.H.
2010-12-11 1:40 ` Jakub Narebski
2010-12-09 21:57 ` [PATCH 12/18] gitweb: Change file handles (in caching) to lexical variables as opposed to globs John 'Warthog9' Hawley
2010-12-10 0:16 ` Jakub Narebski
2010-12-10 0:32 ` Junio C Hamano
2010-12-10 0:47 ` Jakub Narebski
2010-12-10 5:56 ` J.H.
2010-12-09 21:57 ` [PATCH 13/18] gitweb: Add commented url & url hash to page footer John 'Warthog9' Hawley
2010-12-10 0:26 ` Jakub Narebski
2010-12-10 6:10 ` J.H.
2010-12-09 21:57 ` [PATCH 14/18] gitweb: add print_transient_header() function for central header printing John 'Warthog9' Hawley
2010-12-10 0:36 ` Jakub Narebski
2010-12-10 6:18 ` J.H.
2010-12-09 21:57 ` [PATCH 15/18] gitweb: Add show_warning() to display an immediate warning, with refresh John 'Warthog9' Hawley
2010-12-10 1:01 ` Jakub Narebski [this message]
2010-12-10 7:38 ` J.H.
2010-12-10 14:10 ` Jakub Narebski
2010-12-09 21:57 ` [PATCH 16/18] gitweb: When changing output (STDOUT) change STDERR as well John 'Warthog9' Hawley
2010-12-10 1:36 ` Jakub Narebski
2010-12-12 5:25 ` J.H.
2010-12-12 15:17 ` Jakub Narebski
2010-12-09 21:57 ` [PATCH 17/18] gitweb: Prepare for cached error pages & better error page handling John 'Warthog9' Hawley
2010-12-10 1:49 ` Jakub Narebski
2010-12-10 8:33 ` J.H.
2010-12-10 20:33 ` Jakub Narebski
2010-12-09 21:57 ` [PATCH 18/18] gitweb: Add better error handling for gitweb caching John 'Warthog9' Hawley
2010-12-10 1:56 ` Jakub Narebski
2010-12-09 23:26 ` [PATCH 00/18] Gitweb caching v8 Jakub Narebski
2010-12-10 0:43 ` J.H.
2010-12-10 1:27 ` Jakub Narebski
2010-12-10 0:39 ` Junio C Hamano
2010-12-10 0:45 ` J.H.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m3zksezbkm.fsf@localhost.localdomain \
--to=jnareb@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=warthog9@eaglescrag$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox