Re: [PATCH v2 6/8] gitweb: Push formatting diff lines to print_diff_chunk()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jakub Narebski <jnareb@xxxxxxxxx> 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 <michal.kiedrowicz@xxxxxxxxx>
> > Acked-by: Jakub Narębski <jnareb@xxxxxxxxx>
> > ---
> >  gitweb/gitweb.perl |   25 ++++++++++++-------------
> >  1 files changed, 12 insertions(+), 13 deletions(-)
> 
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]