From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Tino Calancha Newsgroups: gmane.emacs.bugs 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 Message-ID: <871swzjeza.fsf@gmail.com> References: <87k2at2t28.fsf@gmail.com> <36cb0896-f437-41f6-92d1-1f8897ff141d@default> <87vauclh42.fsf@gmail.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: blaine.gmane.org 1482477197 10307 195.159.176.226 (23 Dec 2016 07:13:17 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Fri, 23 Dec 2016 07:13:17 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.0.50 (gnu/linux) Cc: 25243@debbugs.gnu.org, Tino Calancha To: Drew Adams Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri Dec 23 08:13:13 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cKK2R-0001j7-Kw for geb-bug-gnu-emacs@m.gmane.org; Fri, 23 Dec 2016 08:13:11 +0100 Original-Received: from localhost ([::1]:37703 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cKK2U-0001zZ-5v for geb-bug-gnu-emacs@m.gmane.org; Fri, 23 Dec 2016 02:13:14 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:58775) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cKK2N-0001z7-Dj for bug-gnu-emacs@gnu.org; Fri, 23 Dec 2016 02:13:08 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cKK2I-0001Qk-FN for bug-gnu-emacs@gnu.org; Fri, 23 Dec 2016 02:13:07 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:36608) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1cKK2I-0001Qb-As for bug-gnu-emacs@gnu.org; Fri, 23 Dec 2016 02:13:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1cKK2H-0004bN-Ug for bug-gnu-emacs@gnu.org; Fri, 23 Dec 2016 02:13:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Tino Calancha Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 23 Dec 2016 07:13:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 25243 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 25243-submit@debbugs.gnu.org id=B25243.148247717617678 (code B ref 25243); Fri, 23 Dec 2016 07:13:01 +0000 Original-Received: (at 25243) by debbugs.gnu.org; 23 Dec 2016 07:12:56 +0000 Original-Received: from localhost ([127.0.0.1]:52007 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cKK2C-0004b4-9N for submit@debbugs.gnu.org; Fri, 23 Dec 2016 02:12:56 -0500 Original-Received: from mail-pf0-f195.google.com ([209.85.192.195]:34838) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cKK28-0004ap-MM for 25243@debbugs.gnu.org; Fri, 23 Dec 2016 02:12:54 -0500 Original-Received: by mail-pf0-f195.google.com with SMTP id i88so13488263pfk.2 for <25243@debbugs.gnu.org>; Thu, 22 Dec 2016 23:12:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=VPv87xwfTXx8kcUNOOB3spLblz8Q29uEGw62pkzTd9g=; b=ESkHFPBXBPyTt61foarRcDNxjlccVisnr6YHvzp175kea9/UjD00saMh50c6NReKRl FH2xmeM3ouxLxFGr3BWxC2wqk1Zofx8tyXI5nwxi16u1M7gKx2Yi+KixUTEM98iU2MEf gEVL+KFugHFmFSh1bmOP/e1PHo9fYaf6qFVgmuvWt1MZW7Cuq4UE6F9F0PwK5CLxAtUM CQ1Bc/wyzmorTpug19thQDDSch/5Omz7s7bBXAa3mpitq2t6Wy5AdCA582IMszLuRDA8 W3MYD+bgOlKn82EfmfwiNKx8ANiCNVjShIc1QWsId05NoG8GMy13K1+C7qmz66VQwP/b 11sA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=VPv87xwfTXx8kcUNOOB3spLblz8Q29uEGw62pkzTd9g=; b=a7snwBT5iShWTLPYxs/2Ntah3fKzf3ns/CR8CpKvLU60QohP3KnQh0dwdIXLL/MPw0 kSbQH7sTWYNqtWcv+N5viwHik/2MVIDQIgCrV/BmMc0lmqg3om64F8VD0bubVfM9Mdar +St+s7tOjX0VqzRev1soB2sV1xtvLUN7UtizYFzZDodS2Tpwa3K+zyp2WKvTTNprXB7H WYJFBm/WGDqKvrRhklLhGRm+gqba7wvaWxprw67yEkP8fNCy3sJDbQHOf+ld0s5ci2Bi 9CrX2JyI0T4tIq8aPtcHrxH3e279vRQidQGbBW/DrqoYRkkYHclYGjf9fbtg0RQtzki/ My1g== X-Gm-Message-State: AIkVDXJbAkiIFRG5vT9Otfugs9wngd6q9uIz50zJQR9wmAltppi94YSvTXbQV9aQntb5vQ== X-Received: by 10.99.65.65 with SMTP id o62mr23738288pga.73.1482477166994; Thu, 22 Dec 2016 23:12:46 -0800 (PST) Original-Received: from calancha-pc (177.192.218.133.dy.bbexcite.jp. [133.218.192.177]) by smtp.gmail.com with ESMTPSA id y15sm59830468pgc.43.2016.12.22.23.12.44 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 22 Dec 2016 23:12:46 -0800 (PST) In-Reply-To: (Drew Adams's message of "Thu, 22 Dec 2016 09:22:38 -0800 (PST)") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:127348 Archived-At: Drew Adams 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