Re: [PATCH 8/9] gitweb: Convert output to using indirect file handle
From: Jakub Narebski <hidden>
Date: 2016-06-15 22:48:02
"John 'Warthog9' Hawley" [off-list ref] writes:
This converts the output handling of gitweb to using an indirect file handle. This is in preparation to add the caching layer. This is a slight modification to the way I was originally doing it by passing the output around. This should be a nop and this shouldn't change the behavior of gitweb. This does leave error reporting functions (die_error specifically) continuing to output directly as I want to garauntee those will report their errors regardless of what may be going on with respect to the rest of the output.
Signoff? Compare with my version of this patch: http://repo.or.cz/w/git/jnareb-git.git/commitdiff/0dd15cb3f18e2a26fc834fd3b071e6d3ecc00557 in the gitweb/cache-kernel branch: http://repo.or.cz/w/git/jnareb-git.git/shortlog/refs/heads/gitweb/cache-kernel My commit message looks like the following: .... gitweb: Print to explicit filehandle (preparing for caching) This means replacing print <something>; by print {$out} <something>; and binmode STDOUT, <layer>; by binmode $out, <layer>; where $out is global variable set to \*STDOUT at the beginning of gitweb, but after reading gitweb config. This way it would be simple to e.g. tie output filehandle or use PerlIO layers to simultaneously write to standard output and to some specified file (like "tee" utility does), or redirect output to a scalar, or a file. die_error (re)sets $out to \*STDOUT; we would (probably) want to treat errors in a special way, and do not cache them. The only other differences are reindent of continued lines (if needed), and sometimes word-wrapping lines which this change made too long. Signed-off-by: Jakub Narebski <redacted> ....
quoted hunk ↗ jump to hunk
--- gitweb/gitweb.perl | 880 ++++++++++++++++++++++++++-------------------------- 1 files changed, 448 insertions(+), 432 deletions(-)diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index c4a177d..8bb323c 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl@@ -450,6 +450,13 @@ our %feature = ( 'default' => [0]}, ); +# Basic file handler for all of gitweb, there are two of them. The first +# is the basic text/html file handler which is used for everything other +# then the binary files, that uses a separate file handler though +# these are both set to STDOUT for the time being. +our $output_handler = *STDOUT; +our $output_handler_bin = *STDOUT; +
First it is not file handleR, but filehandle.
Second, there is no need for separate filehandle for binary files, if
you do it correctly (i.e. call binmode on filehandle, and not on
STDOUT). When caching is enabled, and 'print {$output_handle} <sth>'
prints to in-memory file (or even directly to cache file) it would do
conversion, so when reading from cache file we can dump it raw, in
binary mode.
Third, wouldn't it be better to use shorter variable name, e.g. $out
or $oh, instead of $output_handle? We would be able to align print(f)
statements without making lines much longer.
Fourth, there is slight difference between
our $out = *STDOUT;
and
out $out = \*STDOUT;
In the former we have global variable, in latter we have indirect
filehandle. CGI::Cache uses the latter form, IIRC.
quoted hunk ↗ jump to hunk
@@ -3313,7 +3320,7 @@ EOF if ($use_pathinfo) { $action .= "/".esc_url($project); } - print $cgi->startform(-method => "get", -action => $action) . + print {$output_handler} $cgi->startform(-method => "get", -action => $action) . "<div class=\"search\">\n" . (!$use_pathinfo && $cgi->input({-name=>"p", -value=>$project, -type=>"hidden"}) . "\n") .
Here for example after change gitweb source stops being nicely aligned. OTOH it makes for bigger patch. In my version I did realign. You can always check for true differences with "diff -w". -- Jakub Narebski Poland ShadeHawk on #git