Michał Kiedrowicz <michal.kiedrowicz@xxxxxxxxx> writes:
> The highlightning of combined diffs is currently disabled. This is
> because output from a combined diff is much harder to highlight because
> it's not obvious which removed and added lines should be compared.
>
> Moreover, code that compares added and removed lines doesn't care about
> combined diffs. It only skips first +/- character, treating second +/-
> as a line content.
>
> Let's start with a simple case: only highlight changes that come from
> one parent, i.e. when every removed line has a corresponding added line
> for the same parent. This way the highlightning cannot get wrong. For
> example, following diffs would be highlighted:
>
> - removed line for first parent
> + added line for first parent
> context line
> -removed line for second parent
> +added line for second parent
>
> or
>
> - removed line for first parent
> -removed line for second parent
> + added line for first parent
> +added line for second parent
>
> but following output will not:
>
> - removed line for first parent
> -removed line for second parent
> +added line for second parent
> ++added line for both parents
>
That is a very reasonable approach.
> Further changes may introduce more intelligent approach that better
> handles combined diffs.
>
> Signed-off-by: Michał Kiedrowicz <michal.kiedrowicz@xxxxxxxxx>
> ---
> gitweb/gitweb.perl | 40 +++++++++++++++++++++++++++++++++++++---
> 1 files changed, 37 insertions(+), 3 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 1a5b454..2b6cb9e 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -4944,13 +4944,16 @@ sub esc_html_mark_range {
>
> # Format removed and added line, mark changed part and HTML-format them.
> sub format_rem_add_line {
> - my ($rem, $add) = @_;
> + my ($rem, $add, $is_combined) = @_;
> my @r = split(//, $rem);
> my @a = split(//, $add);
> my ($esc_rem, $esc_add);
> my ($prefix, $suffix_rem, $suffix_add) = (1, $#r, $#a);
> my ($prefix_is_space, $suffix_is_space) = (1, 1);
>
> + # In combined diff we must ignore two +/- characters.
> + $prefix = 2 if ($is_combined);
> +
Errr... actually number of prefix is equalt to number of parents, so
it might be in case of octopus merge more than 2.
> while ($prefix < @r && $prefix < @a) {
> last if ($r[$prefix] ne $a[$prefix]);
>
> @@ -4988,11 +4991,42 @@ sub format_ctx_rem_add_lines {
> my ($ctx, $rem, $add, $is_combined) = @_;
> my (@new_ctx, @new_rem, @new_add);
> my $num_add_lines = @$add;
> + my $can_highlight;
> +
> + # Highlight if every removed line has a corresponding added line.
> + if ($num_add_lines > 0 && $num_add_lines == @$rem) {
> + $can_highlight = 1;
> +
> + # Highlight lines in combined diff only if the chunk contains
> + # diff between the same version, e.g.
> + #
> + # - a
> + # - b
> + # + c
> + # + d
> + #
> + # Otherwise the highlightling would be confusing.
> + if ($is_combined) {
> + for (my $i = 0; $i < $num_add_lines; $i++) {
> + my $prefix_rem = substr($rem->[$i], 0, 2);
> + my $prefix_add = substr($add->[$i], 0, 2);
> +
> + $prefix_rem =~ s/-/+/g;
> +
> + if ($prefix_rem ne $prefix_add) {
> + $can_highlight = 0;
> + last;
Nb. this assumes that we cannot refine and highlight something like
this:
# - a
# - b
# + c
# ++ d
But perhaps this is better left for future improvemnt.
> + }
> + }
> + }
> + } else {
> + $can_highlight = 0;
> + }
This 'else' would be not necessary if $can_highlight was initialized
to 0.
>
> - if (!$is_combined && $num_add_lines > 0 && $num_add_lines == @$rem) {
> + if ($can_highlight) {
> for (my $i = 0; $i < $num_add_lines; $i++) {
> my ($line_rem, $line_add) = format_rem_add_line(
> - $rem->[$i], $add->[$i]);
> + $rem->[$i], $add->[$i], $is_combined);
> push @new_rem, $line_rem;
> push @new_add, $line_add;
O.K.
--
Jakub Narębski
--
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