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

[Tino Calancha <tino.calancha@gmail.com>]

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
>