From: Tino Calancha <tino.calancha@gmail.com>
To: Drew Adams <drew.adams@oracle.com>
Cc: 25243@debbugs.gnu.org, Tino Calancha <tino.calancha@gmail.com>
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 [thread overview]
Message-ID: <871swzjeza.fsf@gmail.com> (raw)
In-Reply-To: <f7fca0cd-097f-4944-a7b3-74241d233588@default> (Drew Adams's message of "Thu, 22 Dec 2016 09:22:38 -0800 (PST)")
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.")
+
\f
;;; 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
next prev parent reply other threads:[~2016-12-23 7:12 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-21 15:35 bug#25243: 26.0.50; ffap-guesser very slow w/ region active in large diff files Tino Calancha
2016-12-21 16:22 ` Drew Adams
2016-12-22 4:31 ` Tino Calancha
2016-12-22 17:22 ` Drew Adams
2016-12-23 7:12 ` Tino Calancha [this message]
2016-12-23 15:41 ` Drew Adams
2016-12-24 2:53 ` Tino Calancha
2016-12-30 6:41 ` Tino Calancha
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=871swzjeza.fsf@gmail.com \
--to=tino.calancha@gmail.com \
--cc=25243@debbugs.gnu.org \
--cc=drew.adams@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.