Re: [PATCH v3 03/15] diff --color-moved: factor out function

"Phillip Wood via GitGitGadget" [off-list ref] writes:

From: Phillip Wood <redacted>

This code is quite heavily indented and having it in its own function
simplifies an upcoming change.

And this should show as "moved" lines correctly in the output from
"log -p --color-moved -w"?

... not really.  There is an unfortunate artificial line wrapping in
the original, which was unwrapped by this move, so the blocks do not
exactly match.

Hi Junio

On 28/10/2021 22:51, Junio C Hamano wrote:

"Phillip Wood via GitGitGadget" [off-list ref] writes:

quoted

From: Phillip Wood <redacted>

This code is quite heavily indented and having it in its own function
simplifies an upcoming change.

And this should show as "moved" lines correctly in the output from
"log -p --color-moved -w"?

rather that "-w" one can use "--color-moved-ws=allow-indentation-change" 
to highlight moved lines where the indentation has changed. It took me a 
while to realize why "-w" does not do anything here but it is because 
the lines are moved as well as having their indentation changed.

... not really.  There is an unfortunate artificial line wrapping in
the original, which was unwrapped by this move, so the blocks do not
exactly match.

Yes that's a shame, it seemed overkill to have one commit moving the 
code as is and then another reformatting it. All of the moved lines 
apart from the one that is unwrapped are highlighted with 
"--color-moved-ws=allow-indentation-change".

Best Wishes

Phillip

quoted

Signed-off-by: Phillip Wood <redacted>
---
  diff.c | 51 ++++++++++++++++++++++++++++++++++-----------------
  1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/diff.c b/diff.c
index bd8e4ec9757..09af94e018c 100644
--- a/diff.c
+++ b/diff.c

@@ -1098,6 +1098,38 @@ static int shrink_potential_moved_blocks(struct moved_block *pmb,
  	return rp + 1;
  }
  
+static void fill_potential_moved_blocks(struct diff_options *o,
+					struct hashmap *hm,
+					struct moved_entry *match,
+					struct emitted_diff_symbol *l,
+					struct moved_block **pmb_p,
+					int *pmb_alloc_p, int *pmb_nr_p)
+
+{
+	struct moved_block *pmb = *pmb_p;
+	int pmb_alloc = *pmb_alloc_p, pmb_nr = *pmb_nr_p;
+
+	/*
+	 * The current line is the start of a new block.
+	 * Setup the set of potential blocks.
+	 */
+	hashmap_for_each_entry_from(hm, match, ent) {
+		ALLOC_GROW(pmb, pmb_nr + 1, pmb_alloc);
+		if (o->color_moved_ws_handling &
+		    COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) {
+			if (compute_ws_delta(l, match->es, &(pmb[pmb_nr]).wsd))
+				pmb[pmb_nr++].match = match;
+		} else {
+			pmb[pmb_nr].wsd = 0;
+			pmb[pmb_nr++].match = match;
+		}
+	}
+
+	*pmb_p = pmb;
+	*pmb_alloc_p = pmb_alloc;
+	*pmb_nr_p = pmb_nr;
+}
+
  /*
   * If o->color_moved is COLOR_MOVED_PLAIN, this function does nothing.
   *

@@ -1198,23 +1230,8 @@ static void mark_color_as_moved(struct diff_options *o,
  		pmb_nr = shrink_potential_moved_blocks(pmb, pmb_nr);
  
  		if (pmb_nr == 0) {
-			/*
-			 * The current line is the start of a new block.
-			 * Setup the set of potential blocks.
-			 */
-			hashmap_for_each_entry_from(hm, match, ent) {
-				ALLOC_GROW(pmb, pmb_nr + 1, pmb_alloc);
-				if (o->color_moved_ws_handling &
-				    COLOR_MOVED_WS_ALLOW_INDENTATION_CHANGE) {
-					if (compute_ws_delta(l, match->es,
-							     &pmb[pmb_nr].wsd))
-						pmb[pmb_nr++].match = match;
-				} else {
-					pmb[pmb_nr].wsd = 0;
-					pmb[pmb_nr++].match = match;
-				}
-			}
-
+			fill_potential_moved_blocks(
+				o, hm, match, l, &pmb, &pmb_alloc, &pmb_nr);
  			if (adjust_last_block(o, n, block_length) &&
  			    pmb_nr && last_symbol != l->s)
  				flipped_block = (flipped_block + 1) % 2;