Jakub Narebski <jnareb@xxxxxxxxx> wrote:
> Michał Kiedrowicz <michal.kiedrowicz@xxxxxxxxx> writes:
>
> > Now we're ready to insert highlightning to diff output.
> >
> > The call to untabify() remains in the main loop in print_diff_chunk().
> > The motivation is that it must be called before any call to esc_html()
> > (because that converts spaces to ) and to call it only once.
> >
> > This is a refactoring patch. It's not meant to change gitweb output.
>
> I'm not sure about this patch.
>
> First, in my opinion it doesn't make much sense standalone, and not
> squashed with subsequent patch.
True. I wanted to separate "preparation" from "adding new feature" but
maybe went few steps too far.
>
> Second, it makes format_diff_line an odd duck among all other format_*
> subroutines in that it post-processes HTML output, rather than
> generating HTML from data.
>
> Why "diff refinement highlighting" cannot be part of format_diff_line()?
> If it does need additional data, just pass it as additional parameters
> to this subroutine.
>
> Another solution could be to borrow idea from stalled and inactive
> committags feature, namely that parts that are HTML are to be passed
> as scalar reference (\$str), while plain text (unescaped yet) is to be
> passed as string ($str).
I'll look into it.
>
> > -# process patch (diff) line (not to be used for diff headers),
> > -# returning HTML-formatted (but not wrapped) line
> > +# wrap patch (diff) line into a <div> (not to be used for diff headers),
> > +# the line must be esc_html()'ed
> > sub format_diff_line {
>
> I just don't like this error-prone "the line must be esc_html()'ed".
>
> > +# HTML-format diff context, removed and added lines.
> > +sub format_ctx_rem_add_lines {
> > + my ($ctx, $rem, $add) = @_;
> > + my (@new_ctx, @new_rem, @new_add);
> > +
> > + @new_ctx = map { format_diff_line(esc_html($_, -nbsp=>1), 'ctx') } @$ctx;
> > + @new_rem = map { format_diff_line(esc_html($_, -nbsp=>1), 'rem') } @$rem;
> > + @new_add = map { format_diff_line(esc_html($_, -nbsp=>1), 'add') } @$add;
> > +
> > + return (\@new_ctx, \@new_rem, \@new_add);
> > +}
> > +
> > # Print context lines and then rem/add lines.
> > sub print_diff_lines {
> > my ($ctx, $rem, $add, $diff_style, $is_combined) = @_;
> >
> > + ($ctx, $rem, $add) = format_ctx_rem_add_lines($ctx, $rem, $add);
> > +
>
> Nice trick.
>
--
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