From: Drew Adams <drew.adams@oracle.com>
To: Tino Calancha <tino.calancha@gmail.com>, 25243@debbugs.gnu.org
Subject: bug#25243: 26.0.50; ffap-guesser very slow w/ region active in large diff files
Date: Wed, 21 Dec 2016 08:22:19 -0800 (PST) [thread overview]
Message-ID: <36cb0896-f437-41f6-92d1-1f8897ff141d@default> (raw)
In-Reply-To: <87k2at2t28.fsf@gmail.com>
Amusing backstory:
In some of my code I use `ffap-guesser' to pick up
file-name defaults for own my version of `read-file-name'.
I added that pretty much naively, without digging into the
`ffap-guesser' code to check what it does. It seemed to
work pretty well.
Tino sent me a diff file of about 14 KB, which showed the
problem. Other files, even 10 times as large (!), didn't
necessarily show the problem.
The test case was to use `write-region' in that diff-file
buffer, after `C-x h'. (In context, `write-region' invoked
my version of `read-file-name'.)
Looking closer, I saw that `ffap-guesser' tries to use the
text in the active region to guess a file name, and that it
passes this text around to multiple functions that examine
it, search through it, and massage it (e.g., remove text
properties).
Needless to say, this is problematic for a large active region.
Had the doc string of `ffap-guesser' (and other functions
that call it and that it calls) mentioned that it uses the
active region, I likely would never have used it as I did.
Stefan did add a comment, in Emacs 23, for one call to
`ffap-guesser', which is helpful:
;; Don't use the region here, since it can be something
;; completely unwieldy. If the user wants that, she could
;; use M-w before and then C-y. --Stef
But it's not just for that occurrence that the problem can
exist. And putting that info in a doc string would be more
helpful than just in a comment. It's not just a message to
Emacs implementers; it's something that users of the ffap
functions should know about.
Preventing the problem in the first place, as Tino's patch
tries to do, is even better than just documenting the gotcha.
The ffap.el code does prevent the problem itself for some
uses of `ffap-guesser' (e.g. `ffap-prompter'), but it is
in `ffap-guesser' itself (or `ffap-file-at-point' or
`ffap-string-at-point') that this should be done.
Wrt the proposed patch:
1. It is `ffap-string-at-point' that picks up the active
region text as a string and removes its properties.
Since that is what is slow, I think it is in that
function that preventing this from happening should
occur. The patch tries to control this only in
`ffap-file-at-point', and it does so _after_ calling
`ffap-string-at-point', which is too late (AFAICS).
I think that `ffap-string-at-point' should control
this. It should not try to pick up a file name from
a 14 KB diff buffer.
2. I'm not sure that a user option is really what's called
for here. I'd suggest a defvar, but only because Emacs
Dev does not really like programs to bind option values
(I have little problem with that, myself), and that is
the main place where I expect the value to be changed:
programmatically.
Thanks, Tino, for finding this bug and reporting it.
next prev parent reply other threads:[~2016-12-21 16:22 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 [this message]
2016-12-22 4:31 ` Tino Calancha
2016-12-22 17:22 ` Drew Adams
2016-12-23 7:12 ` Tino Calancha
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=36cb0896-f437-41f6-92d1-1f8897ff141d@default \
--to=drew.adams@oracle.com \
--cc=25243@debbugs.gnu.org \
--cc=tino.calancha@gmail.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.