From: Michał Kiedrowicz Date: Thu, 29 Mar 2012 17:41:53 GMT Subject: Re: [PATCH v2 6/8] gitweb: Push formatting diff lines to print_diff_chunk() Message-ID: <20120329194153.2e1ad827@gmail.com> In-Reply-To: <201203291859.44891.jnareb@gmail.com> Jakub Narebski wrote: > On Fri, 23 Mon 2012, Michał Kiedrowicz wrote: > > > Now git_patchset_body() only calls diff_line_class(), which is removed > > from process_diff_line(). The latter function is renamed to > > format_diff_line() and its output is changed to return only > > HTML-formatted line, which brings it in line with outher format_* > > subroutined. > > > > This slightly changes the order of operations performed on diff lines. > > Before this commit, each read line was formatted and then put to the > > @chunk accumulator. Now, lines are formatted inside print_diff_chunk(), > > This is a bit convoluted description. > > > As I understand it, what happens here is that formatting lines is > pushed down to print_diff_chunk(), closer to the place where we > actually use HTML formatted output. Yes. > > This means that we put raw lines in the @chunk accumulator, rather > than formatted lines. Because we still need to know class (type) > of line when accumulating data to post-process and print, > process_diff_line() subroutine was retired and replaced by > diff_line_class() used in git_patchset_body() and new / resurrected > format_diff_line() used in print_diff_chunk(). > > Isn't it? Very true. > > > A side effect is that we have to pass \%from and \%to down the > callstack. Yes. > > > This is a preparation patch for diff refinement highlightning. It's not > > meant to change gitweb output. > > > This is a very nice refactoring. I was never really comfortable with > the API of process_diff_line(), which was different from all other > subroutines in gitweb, and error prone to call. I wish we used this > solution presented in this commit from the very beginning. > > BTW. I think we can simply squash this commit with previous one; no > need to improve process_diff_line() if we are retiring it. OK, will do. > > > Signed-off-by: Michał Kiedrowicz > > Acked-by: Jakub Narębski > > --- > > gitweb/gitweb.perl | 25 ++++++++++++------------- > > 1 files changed, 12 insertions(+), 13 deletions(-) >