[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#25243: 26.0.50; ffap-guesser very slow w/ region active in large dif
From: |
Tino Calancha |
Subject: |
bug#25243: 26.0.50; ffap-guesser very slow w/ region active in large diff files |
Date: |
Fri, 23 Dec 2016 16:12:41 +0900 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux) |
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