bug#25243: 26.0.50; ffap-guesser very slow w/ region active in large diff files

[Tino Calancha]

Drew Adams <drew.adams@oracle.com> writes:

Thanks for the comments!  I answer below.

>> +(defvar ffap-max-region-length 1024
>> +  "Maximum allowed region length in `ffap-string-at-point'.")
>
> 1. I think it should say "active region".
Agreed.
>
> Very minor (can be ignored): If we say something is not allowed it is
> unclear what happens.  In particular, it can suggest that we raise an
> error. You might want to say here that if the active region is larger
> ... it is considered empty.  (Or just refer to `ffap-string-at-point',
> which you do already.) 
OK.

>> +         (region-len (- (max beg end) (min beg end))))
>> +    (if (or (null ffap-max-region-length)
>> +            (< region-len ffap-max-region-length)) ; Bug#25243.
>> +        (setf ffap-string-at-point-region (list beg end)
>> +              ffap-string-at-point
>> +              (buffer-substring-no-properties beg end))
>> +      (setf ffap-string-at-point-region (list 1 1)
>> +            ffap-string-at-point ""))))
>
> 1. The doc string should say that if the active region is
> larger than `ffap-max-region-length' then those two vars
> are set to ... and ....
I see.  I added some text.  The var `ffap-string-at-point' gets the
returned value of the function with the same name; so it suffices to say
that in that case the func. returns "".  I also mention that
`ffap-string-at-point-region' is set to '(1 1).
> 2. Instead of testing whether the max-length var is nil, I'd suggest
> testing it with `natnump', to take care of the unexpected case where
> it might get assigned a non-number.
Yes, `natnump' is a better choice.

Following is the updated patch:
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
diff --git a/lisp/ffap.el b/lisp/ffap.el
index 3d7cebadcf..d91f50e060 100644
--- a/lisp/ffap.el
+++ b/lisp/ffap.el
@@ -203,6 +203,11 @@ ffap-foo-at-bar-prefix
                 )
   :group 'ffap)
 
+(defvar ffap-max-region-length 1024
+  "Maximum active region length.
+When the region is active and larger than this value,
+`ffap-string-at-point' returns an empty string.")
+
 
 ;;; Peanut Gallery (More User Variables):
 ;;
@@ -1101,8 +1106,10 @@ ffap-string-at-point
 string syntax parameters in `ffap-string-at-point-mode-alist'.
 If MODE is not found, we use `file' instead of MODE.
 If the region is active, return a string from the region.
-Sets the variable `ffap-string-at-point' and the variable
-`ffap-string-at-point-region'."
+Set the variable `ffap-string-at-point' and the variable
+`ffap-string-at-point-region'.
+When the region is active and larger than `ffap-max-region-length',
+return an empty string, and set `ffap-string-at-point-region' to '(1 1)."
   (let* ((args
          (cdr
           (or (assq (or mode major-mode) ffap-string-at-point-mode-alist)
@@ -1119,11 +1126,15 @@ ffap-string-at-point
                (save-excursion
                  (skip-chars-forward (car args))
                  (skip-chars-backward (nth 2 args) pt)
-                 (point)))))
-    (setq ffap-string-at-point
-         (buffer-substring-no-properties
-          (setcar ffap-string-at-point-region beg)
-          (setcar (cdr ffap-string-at-point-region) end)))))
+                 (point))))
+         (region-len (- (max beg end) (min beg end))))
+    (if (or (not (natnump ffap-max-region-length))
+            (< region-len ffap-max-region-length)) ; Bug#25243.
+        (setf ffap-string-at-point-region (list beg end)
+              ffap-string-at-point
+              (buffer-substring-no-properties beg end))
+      (setf ffap-string-at-point-region (list 1 1)
+            ffap-string-at-point ""))))
 
 (defun ffap-string-around ()
   ;; Sometimes useful to decide how to treat a string.

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.4)
 of 2016-12-21
Repository revision: 8661313efd5fd5b0a27fe82f276a1ff862646424