From: Junio C Hamano <gitster@pobox.com>
Date: Sun, 27 Aug 2017 05:55:04 +0000 (-0700)
Subject: Merge branch 'jt/diff-color-move-fix'
X-Git-Tag: v2.15.0-rc0~129
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/0b9635847944f97079fc6c2283063cd3a0ed0271?ds=inline;hp=-c

Merge branch 'jt/diff-color-move-fix'

A handful of bugfixes and an improvement to "diff --color-moved".

* jt/diff-color-move-fix:
diff: define block by number of alphanumeric chars
diff: respect MIN_BLOCK_LENGTH for last block
diff: avoid redundantly clearing a flag
---

0b9635847944f97079fc6c2283063cd3a0ed0271
diff --combined Documentation/diff-options.txt
index b1ab96477b,b8c881605c..a88c76741e
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@@ -254,13 -254,11 +254,11 @@@ plain:
  	moved line, but it is not very useful in a review to determine
  	if a block of code was moved without permutation.
  zebra::
- 	Blocks of moved code are detected greedily. The detected blocks are
+ 	Blocks of moved text of at least 20 alphanumeric characters
+ 	are detected greedily. The detected blocks are
  	painted using either the 'color.diff.{old,new}Moved' color or
  	'color.diff.{old,new}MovedAlternative'. The change between
- 	the two colors indicates that a new block was detected. If there
- 	are fewer than 3 adjacent moved lines, they are not marked up
- 	as moved, but the regular colors 'color.diff.{old,new}' will be
- 	used.
+ 	the two colors indicates that a new block was detected.
  dimmed_zebra::
  	Similar to 'zebra', but additional dimming of uninteresting parts
  	of moved code is performed. The bordering lines of two adjacent
@@@ -336,14 -334,15 +334,14 @@@ ifndef::git-format-patch[
  	with --exit-code.
  
  --ws-error-highlight=<kind>::
 -	Highlight whitespace errors on lines specified by <kind>
 -	in the color specified by `color.diff.whitespace`.  <kind>
 -	is a comma separated list of `old`, `new`, `context`.  When
 -	this option is not given, only whitespace errors in `new`
 -	lines are highlighted.  E.g. `--ws-error-highlight=new,old`
 -	highlights whitespace errors on both deleted and added lines.
 -	`all` can be used as a short-hand for `old,new,context`.
 -	The `diff.wsErrorHighlight` configuration variable can be
 -	used to specify the default behaviour.
 +	Highlight whitespace errors in the `context`, `old` or `new`
 +	lines of the diff.  Multiple values are separated by comma,
 +	`none` resets previous values, `default` reset the list to
 +	`new` and `all` is a shorthand for `old,new,context`.  When
 +	this option is not given, and the configuration variable
 +	`diff.wsErrorHighlight` is not set, only whitespace errors in
 +	`new` lines are highlighted. The whitespace errors are colored
 +	whith `color.diff.whitespace`.
  
  endif::git-format-patch[]
  
@@@ -427,7 -426,7 +425,7 @@@ endif::git-log[
  	the diff between the preimage and `/dev/null`. The resulting patch
  	is not meant to be applied with `patch` or `git apply`; this is
  	solely for people who want to just concentrate on reviewing the
 -	text after the change. In addition, the output obviously lack
 +	text after the change. In addition, the output obviously lacks
  	enough information to apply such a patch in reverse, even manually,
  	hence the name of the option.
  +
diff --combined diff.c
index 8c7e360b69,c50fcc7ea5..ac4023d30b
--- a/diff.c
+++ b/diff.c
@@@ -357,6 -357,9 +357,6 @@@ int git_diff_ui_config(const char *var
  		return 0;
  	}
  
 -	if (git_color_config(var, value, cb) < 0)
 -		return -1;
 -
  	return git_diff_basic_config(var, value, cb);
  }
  
@@@ -464,6 -467,8 +464,6 @@@ static struct diff_tempfile 
  	struct tempfile tempfile;
  } diff_temp[2];
  
 -typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);
 -
  struct emit_callback {
  	int color_diff;
  	unsigned ws_rule;
@@@ -471,6 -476,7 +471,6 @@@
  	int blank_at_eof_in_postimage;
  	int lno_in_preimage;
  	int lno_in_postimage;
 -	sane_truncate_fn truncate;
  	const char **label_path;
  	struct diff_words_data *diff_words;
  	struct diff_options *opt;
@@@ -855,6 -861,38 +855,38 @@@ static int shrink_potential_moved_block
  	return rp + 1;
  }
  
+ /*
+  * If o->color_moved is COLOR_MOVED_PLAIN, this function does nothing.
+  *
+  * Otherwise, if the last block has fewer alphanumeric characters than
+  * COLOR_MOVED_MIN_ALNUM_COUNT, unset DIFF_SYMBOL_MOVED_LINE on all lines in
+  * that block.
+  *
+  * The last block consists of the (n - block_length)'th line up to but not
+  * including the nth line.
+  *
+  * NEEDSWORK: This uses the same heuristic as blame_entry_score() in blame.c.
+  * Think of a way to unify them.
+  */
+ static void adjust_last_block(struct diff_options *o, int n, int block_length)
+ {
+ 	int i, alnum_count = 0;
+ 	if (o->color_moved == COLOR_MOVED_PLAIN)
+ 		return;
+ 	for (i = 1; i < block_length + 1; i++) {
+ 		const char *c = o->emitted_symbols->buf[n - i].line;
+ 		for (; *c; c++) {
+ 			if (!isalnum(*c))
+ 				continue;
+ 			alnum_count++;
+ 			if (alnum_count >= COLOR_MOVED_MIN_ALNUM_COUNT)
+ 				return;
+ 		}
+ 	}
+ 	for (i = 1; i < block_length + 1; i++)
+ 		o->emitted_symbols->buf[n - i].flags &= ~DIFF_SYMBOL_MOVED_LINE;
+ }
+ 
  /* Find blocks of moved code, delegate actual coloring decision to helper */
  static void mark_color_as_moved(struct diff_options *o,
  				struct hashmap *add_lines,
@@@ -890,20 -928,13 +922,13 @@@
  		}
  
  		if (!match) {
- 			if (block_length < COLOR_MOVED_MIN_BLOCK_LENGTH &&
- 			    o->color_moved != COLOR_MOVED_PLAIN) {
- 				for (i = 0; i < block_length + 1; i++) {
- 					l = &o->emitted_symbols->buf[n - i];
- 					l->flags &= ~DIFF_SYMBOL_MOVED_LINE;
- 				}
- 			}
+ 			adjust_last_block(o, n, block_length);
  			pmb_nr = 0;
  			block_length = 0;
  			continue;
  		}
  
  		l->flags |= DIFF_SYMBOL_MOVED_LINE;
- 		block_length++;
  
  		if (o->color_moved == COLOR_MOVED_PLAIN)
  			continue;
@@@ -933,11 -964,17 +958,17 @@@
  			}
  
  			flipped_block = (flipped_block + 1) % 2;
+ 
+ 			adjust_last_block(o, n, block_length);
+ 			block_length = 0;
  		}
  
+ 		block_length++;
+ 
  		if (flipped_block)
  			l->flags |= DIFF_SYMBOL_MOVED_LINE_ALT;
  	}
+ 	adjust_last_block(o, n, block_length);
  
  	free(pmb);
  }
@@@ -1965,6 -2002,8 +1996,6 @@@ static unsigned long sane_truncate_line
  	unsigned long allot;
  	size_t l = len;
  
 -	if (ecb->truncate)
 -		return ecb->truncate(line, len);
  	cp = line;
  	allot = l;
  	while (0 < l) {
@@@ -2809,7 -2848,7 +2840,7 @@@ static void show_dirstat_by_line(struc
  			 * bytes per "line".
  			 * This is stupid and ugly, but very cheap...
  			 */
 -			damage = (damage + 63) / 64;
 +			damage = DIV_ROUND_UP(damage, 64);
  		ALLOC_GROW(dir.files, dir.nr + 1, dir.alloc);
  		dir.files[dir.nr].name = file->name;
  		dir.files[dir.nr].changed = damage;