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: Sat, 24 Dec 2016 11:53:27 +0900 Message-ID: <87k2aqdom0.fsf@gmail.com> References: <87k2at2t28.fsf@gmail.com> <36cb0896-f437-41f6-92d1-1f8897ff141d@default> <87vauclh42.fsf@gmail.com> <871swzjeza.fsf@gmail.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: blaine.gmane.org 1482548061 4280 195.159.176.226 (24 Dec 2016 02:54:21 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sat, 24 Dec 2016 02:54:21 +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 Sat Dec 24 03:54:16 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 1cKcTN-00008D-Ix for geb-bug-gnu-emacs@m.gmane.org; Sat, 24 Dec 2016 03:54:13 +0100 Original-Received: from localhost ([::1]:41711 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cKcTS-0002KB-97 for geb-bug-gnu-emacs@m.gmane.org; Fri, 23 Dec 2016 21:54:18 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:49847) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cKcTF-0002FX-Nl for bug-gnu-emacs@gnu.org; Fri, 23 Dec 2016 21:54:07 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cKcTC-0001mt-KN for bug-gnu-emacs@gnu.org; Fri, 23 Dec 2016 21:54:05 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:37687) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1cKcTC-0001mm-GC for bug-gnu-emacs@gnu.org; Fri, 23 Dec 2016 21:54:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1cKcTC-0003cT-91 for bug-gnu-emacs@gnu.org; Fri, 23 Dec 2016 21:54:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Tino Calancha Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 24 Dec 2016 02:54:02 +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.148254802113886 (code B ref 25243); Sat, 24 Dec 2016 02:54:02 +0000 Original-Received: (at 25243) by debbugs.gnu.org; 24 Dec 2016 02:53:41 +0000 Original-Received: from localhost ([127.0.0.1]:53086 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cKcSr-0003bt-3F for submit@debbugs.gnu.org; Fri, 23 Dec 2016 21:53:41 -0500 Original-Received: from mail-pg0-f65.google.com ([74.125.83.65]:33265) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cKcSo-0003be-RO for 25243@debbugs.gnu.org; Fri, 23 Dec 2016 21:53:39 -0500 Original-Received: by mail-pg0-f65.google.com with SMTP id g1so2334182pgn.0 for <25243@debbugs.gnu.org>; Fri, 23 Dec 2016 18:53:38 -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=8x2R4579yOluJuiLq8GOUEirPGDgGV99KbVZkVOTSqs=; b=t+EWlej2pVWRizLD4K7gDqLgGK5Wz1bqQoE25QDF9YpwoXgFbzG6EYmMRf0Kf24OUu GBnY7ULIj8jU1PzBnEytHKlvvuYbYuObDfGedxkStWE2y+Qfg8wcIjmVwt8/Udzvr8sG 0Z36h4rjzi7PptACeQmYRJ2OtqsPRAXvYZ+pZvOe4FZME6CTb21D6KW7z75WUWo1rKqn jwIvVbF9oZGsh8c8Es7RvWqZQg9Y/DY7A0C73KbWTk88Nh3ihODLlGX2Rp0KoNYBmh5X E3ty2M7BKiyhV8z3WErMle80Kth9bQcqF9jibgqA+wgYtiIj4R+r0OyerOiZXJAsEr91 4ZVg== 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=8x2R4579yOluJuiLq8GOUEirPGDgGV99KbVZkVOTSqs=; b=d1v0Yt6zDDkOri5MmE1OX39gfjCMdvEDrMM6yEHjEEbwmGsgvx10JYWzIsYR6R4LWt /IQgepOURs/n+VeyzzGfilnSF1mPG8758renqvm4Uj6AcMLaAhMa7oMnpf6UoswJgarp p1k2+1BuiEYs3NDL/c5IhuHs9uAjgy3da8OyqKPH7JFgEOcD3b9hSYEjmY/yfADISBve HXrHQSZoLhx8utSLwKu3Qiz+nsTmW4+vuFwF9lcqnrBxoP8sKhccc768IQLR9gyYoVaw ATYlX6rKseEazjqAhV42bPN68DdksMWGyMWXT8a0PXuj6cWMLKlqZ68bsAlmb3/64YJ7 ZW3w== X-Gm-Message-State: AIkVDXLi38pw3jVNemeQE3G0LI+ypz+sduWcGOP9ZVzDomsBO/lOoxwA9Siv77Ar3PF+yA== X-Received: by 10.84.211.7 with SMTP id b7mr34975841pli.83.1482548013020; Fri, 23 Dec 2016 18:53:33 -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 1sm65619303pgp.1.2016.12.23.18.53.31 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 23 Dec 2016 18:53:32 -0800 (PST) In-Reply-To: (Drew Adams's message of "Fri, 23 Dec 2016 07:41:02 -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:127382 Archived-At: Drew Adams writes: > below > >> > 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. > >> + (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 "")))) > > I'd suggest the other way around. What you have lets someone or > some code assign a non-number and get the same slow behavior we > want to avoid. I'd say (and (natnump ...) (< region-len ...)). > > IOW, if it's not a natnump and the size is not smaller than that > number then don't use the region. It sounds reasonable. Thank you. If i don't see further comments in a few days i will push the following patch to the master branch: ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; >From 7e8268b975c8385015769755e8ba1e9854d64da1 Mon Sep 17 00:00:00 2001 From: Tino Calancha Date: Sat, 24 Dec 2016 11:31:53 +0900 Subject: [PATCH] ffap-string-at-point: Limit max length of active region Do not spend time checking large strings which are likely not valid candidates (Bug#25243). * lisp/ffap.el (ffap-max-region-length): New variable. (ffap-string-at-point): Use it. * test/lisp/ffap-tests.el: New test suite. (ffap-tests-25243): Add test for this bug. --- lisp/ffap.el | 25 ++++++++++++++++------- test/lisp/ffap-tests.el | 54 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 7 deletions(-) create mode 100644 test/lisp/ffap-tests.el diff --git a/lisp/ffap.el b/lisp/ffap.el index 3d7cebadcf..99bb65faaf 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 (and (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. diff --git a/test/lisp/ffap-tests.el b/test/lisp/ffap-tests.el new file mode 100644 index 0000000000..61fa891fe7 --- /dev/null +++ b/test/lisp/ffap-tests.el @@ -0,0 +1,54 @@ +;;; ffap-tests.el --- Test suite for ffap.el -*- lexical-binding: t -*- + +;; Copyright (C) 2016 Free Software Foundation, Inc. + +;; Author: Tino Calancha + +;; This file is part of GNU Emacs. + +;; GNU Emacs is free software: you can redistribute it and/or modify +;; it under the terms of the GNU General Public License as published by +;; the Free Software Foundation, either version 3 of the License, or +;; (at your option) any later version. + +;; GNU Emacs is distributed in the hope that it will be useful, +;; but WITHOUT ANY WARRANTY; without even the implied warranty of +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;; GNU General Public License for more details. + +;; You should have received a copy of the GNU General Public License +;; along with GNU Emacs. If not, see . + +;;; Commentary: + +;;; Code: + +(require 'ert) +(require 'ffap) + +(ert-deftest ffap-tests-25243 () + "Test for http://debbugs.gnu.org/25243 ." + (let ((file (make-temp-file "test-Bug#25243"))) + (unwind-protect + (with-temp-file file + (let ((str "diff --git b/lisp/ffap.el a/lisp/ffap.el +index 3d7cebadcf..ad4b70d737 100644 +--- b/lisp/ffap.el ++++ a/lisp/ffap.el +@@ -203,6 +203,9 @@ ffap-foo-at-bar-prefix +")) + (transient-mark-mode 1) + (when (natnump ffap-max-region-length) + (insert + (concat + str + (make-string ffap-max-region-length #xa) + (format "%s ENDS HERE" file))) + (mark-whole-buffer) + (should (equal "" (ffap-string-at-point))) + (should (equal '(1 1) ffap-string-at-point-region))))) + (and (file-exists-p file) (delete-file file))))) + +(provide 'ffap-tests) + +;;; ffap-tests.el ends here -- 2.11.0 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; In GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.5) of 2016-12-23 Repository revision: eff901b8a39f42ddedf4c1db833b9071cae5962f