bug#25410: 26.0.50; Refine an unified diff hunk only if adds lines

[Tino Calancha]

On Wed, 11 Jan 2017, npostavs@users.sourceforge.net wrote:

> Tino Calancha <tino.calancha@gmail.com> writes:
>
> > From a performance point of view, current code in the case where
> > the hunk just adds lines is not as patological as the opposite one.
> > M-! git diff ef8c9f8^ ef8c9f8 RET
>
> By the way, with Emacs 25.1, I notice trying to refine this gives me a
> regex stack overflow error.  Probably my recent changes to the regex
> stack limits allow Emacs to spend more time on this instead of
> overflowing.
Yeah, your commit has revealed such nasty regexp's.  Before we get
a stack overflow and the refine of the hunk ends.

> > We might use the first patch in this thread or the following one:
>
> It's probably okay to use the first patch (i.e., don't bother checking
> for delete-only hunks), with an added comment about the asymmetry.  But
> I think it would be better to change diff-refine-hunk to avoid the
> inefficient regex, like this:
I agree with you it's better to not use such heavy regexp matching
too many lines.
Your patch LGTM.  Thank very much.

> From f5ea9e585b535390a69e442d83ecbeec8e8e18d2 Mon Sep 17 00:00:00 2001
> From: Noam Postavsky <npostavs@gmail.com>
> Date: Wed, 11 Jan 2017 23:21:38 -0500
> Subject: [PATCH v1] Avoid inefficient regex in diff-refine-hunk 
(Bug#25410)
> * lisp/vc/diff-mode.el (diff--forward-while-leading-char): New function.
> (diff-refine-hunk): Use it instead of trying to match multiple lines
> with a single lines.
> ---
> lisp/vc/diff-mode.el | 27 +++++++++++++++++----------
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
> index 9dfcd94..915e0b1 100644
> --- a/lisp/vc/diff-mode.el
> +++ b/lisp/vc/diff-mode.el
> @@ -2062,6 +2062,15 @@ diff-refine-preproc
> (declare-function smerge-refine-subst "smerge-mode"
>                   (beg1 end1 beg2 end2 props-c &optional preproc props-r 
props-a))
> +(defun diff--forward-while-leading-char (char bound)
> +  "Move point until reaching a line not starting with CHAR.
> +Return new point, if it was moved."
> +  (let ((pt nil))
> +    (while (and (< (point) bound) (eql (following-char) char))
> +      (forward-line 1)
> +      (setq pt (point)))
> +    pt))
> +
> (defun diff-refine-hunk ()
>   "Highlight changes of hunk at point at a finer granularity."
>   (interactive)
> @@ -2081,16 +2090,14 @@ diff-refine-hunk
>       (goto-char beg)
>       (pcase style
>         (`unified
> -         (while (re-search-forward
> -                 (eval-when-compile
> -                   (let ((no-LF-at-eol-re "\\(?:\\\\.*\n\\)?"))
> -                     (concat "^\\(?:-.*\n\\)+" no-LF-at-eol-re
> -                             "\\(\\)"
> -                             "\\(?:\\+.*\n\\)+" no-LF-at-eol-re)))
> -                 end t)
> -           (smerge-refine-subst (match-beginning 0) (match-end 1)
> -                                (match-end 1) (match-end 0)
> -                                nil 'diff-refine-preproc props-r 
props-a)))
> +         (while (re-search-forward "^-" end t)
> +           (let ((beg-del (progn (beginning-of-line) (point)))
> +                 beg-add end-add)
> +             (when (and (setq beg-add (diff--forward-while-leading-char 
?- end))
> +                        (or (diff--forward-while-leading-char ?\\ end) 
t)
> +                        (setq end-add (diff--forward-while-leading-char 
?+ end)))
> +               (smerge-refine-subst beg-del beg-add beg-add end-add
> +                                    nil 'diff-refine-preproc props-r 
props-a)))))
>         (`context
>          (let* ((middle (save-excursion (re-search-forward "^---")))
>                 (other middle))
> --
> 2.9.3