On Sun, Jan 24, 2010 at 23:14 +0100, Petr Baudis wrote: > On Fri, Jan 15, 2010 at 04:43:32PM -0800, Jakub Narebski wrote: > > 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. > > I totally agree, I was going to comment on this too. Please use > something very short, I think even the three characters might be pushing > it a bit. Having long idiom will make it real pain to both read and > write gitweb code. > > It would still be nice if we could have some way to avoid this hack > entirely. Couldn't we layer PerlIO::via over STDOUT and continue to use > argument-less print as before? First, the original version in v2 series, the one that can be found in 42641b1 (GITWEB - File based caching layer, 2009-12-10) commit in git://git.kernel.org/pub/scm/git/warthog9/gitweb.git gitweb-ml-v2 http://git.kernel.org/?p=git/warthog9/gitweb.git;a=shortlog;h=refs/heads/gitweb-ml-v2 the change was even larger. It was print <something>; replaced by $output .= <something>; and of course make functions that printed sth return formatted output instead. Also with original version you paid the price even if caching was turned off (not used): the whole output had to fit in memory, and there was no streaming, which meant that the whole output had to be generated before it could be displayed (decreasing latency). Second, PerlIO::via is not a good solution. One solution would be to require Capture::Tiny if caching is enabled, and just do $out = capture { $actions{$action}->(); } or even simply (if it is possible with the way 'capture' is prototyped) $out = capture $actions{$action}; Another solution would be to use PerlIO::Util to redirect output to scalar: *STDOUT->push_layer(scalar => \$buffer); # ... *STDOUT->pop_layer(); Yet another solution would be to manipulate *STDOUT directly $stdout = *STDOUT; *STDOUT = $buffer_fh; ... *STDOUT = $stdout; for which, I think, to be done correctly we would have to duplicate (large?) parts of Capture::Tiny. So there is a bit of rule of preservation of difficulty at work. Either we have large patch adding explicit filehandle to all print statements 'print <sth> -> print $out <sth>' but simple code, or have smaller patch but complicated *STDOUT manipulation, or have small patch but rely on non-core CPAN modules present. > I think we (well, in practice, "you" nowadays :) should be less > perfectionist when accepting gitweb contributions, so if the answer is > "it's too hard for me to do right now", that's fine I guess; I know I > can't contribute the code currently. :( True. At least two series ('categories' which I have copy locally in http://repo.or.cz/w/git/jnareb-git.git as gitweb/category, and 'committags' by Marcel M. Cary[1]) that are not yet in gitweb because of that... [1]: http://thread.gmane.org/gmane.comp.version-control.git/100418/focus=133141 -- Jakub Narebski Poland -- 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