| Message ID | 658aec2670c78f9753a5acccab20d3a1741403e6.1635336262.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | diff --color-moved[-ws] speedups | expand |
"Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > 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. > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> > --- > 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;
Hi Junio On 28/10/2021 22:51, Junio C Hamano wrote: > "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> 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 >> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> >> --- >> 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;
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;