Re: [PATCH v1 9/9] diff --color-moved-ws: handle blank lines
- From
- Phillip Wood <phillip.wood@talktalk.net>
- Date
- Nov 21, 2018, 15:49 UTC
- Message-ID
- <ef49bb0e-cf03-a707-f562-595564bd70d8@talktalk.net>
- In-Reply-To
- <CAGZ79kYm-uNWi-3=0fG=PfA3HbT7tKwER=r8fm6UFiy3P=JEmA@mail.gmail.com>
On 20/11/2018 18:05, Stefan Beller wrote:
Show 49 quoted lines
> On Fri, Nov 16, 2018 at 3:04 AM Phillip Wood <phillip.wood@talktalk.net> wrote: >> >> From: Phillip Wood <phillip.wood@dunelm.org.uk> >> >> When using --color-moved-ws=allow-indentation-change allow lines with >> the same indentation change to be grouped across blank lines. For now >> this only works if the blank lines have been moved as well, not for >> blocks that have just had their indentation changed. >> >> This completes the changes to the implementation of >> --color-moved=allow-indentation-change. Running >> >> git diff --color-moved=allow-indentation-change v2.18.0 v2.19.0 >> >> now takes 5.0s. This is a saving of 41% from 8.5s for the optimized >> version of the previous implementation and 66% from the original which >> took 14.6s. >> >> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> >> --- >> >> Notes: >> Changes since rfc: >> - Split these changes into a separate commit. >> - Detect blank lines when processing the indentation rather than >> parsing each line twice. >> - Tweaked the test to make it harder as suggested by Stefan. >> - Added timing data to the commit message. >> >> diff.c | 34 ++++++++++++++++++++++++++++--- >> t/t4015-diff-whitespace.sh | 41 ++++++++++++++++++++++++++++++++++---- >> 2 files changed, 68 insertions(+), 7 deletions(-) >> >> diff --git a/diff.c b/diff.c >> index 89559293e7..072b5bced6 100644 >> --- a/diff.c >> +++ b/diff.c >> @@ -792,9 +792,11 @@ static void moved_block_clear(struct moved_block *b) >> memset(b, 0, sizeof(*b)); >> } >> >> +#define INDENT_BLANKLINE INT_MIN > > Answering my question from the previous patch: > This is why we need to keep the indents signed. > > This patch looks quite nice to read along. > > The whole series looks good to me.
Thanks
> Do we need to update the docs in any way?
I'm not sure, at the moment it does not make any promises about the exact behavior of --color-moved-ws=allow-indentation-change, we could change it to be more explicit but I'm not sure it's worth it.
Thanks for looking over these patches, I'll post a reroll soon based on your comments.
Phillip
> Thanks, > Stefan >