From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?UTF-8?Q?K=C3=A9vin?= Le Gouguec Newsgroups: gmane.emacs.bugs Subject: bug#57733: 29.0.50; Fix "Invalid search bound (wrong side of point)" error in replace-*-in-region Date: Sun, 11 Sep 2022 19:09:04 +0200 Message-ID: <87illtbzyn.fsf@gmail.com> References: <87r10hc35z.fsf@gmail.com> <87illtdgxe.fsf@gnus.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="33809"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: 57733@debbugs.gnu.org To: Lars Ingebrigtsen Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sun Sep 11 19:10:13 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oXQTF-0008cs-FB for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 11 Sep 2022 19:10:13 +0200 Original-Received: from localhost ([::1]:48042 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1oXQTD-0005Rh-UO for geb-bug-gnu-emacs@m.gmane-mx.org; Sun, 11 Sep 2022 13:10:11 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:43416) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oXQT4-0005RX-Ps for bug-gnu-emacs@gnu.org; Sun, 11 Sep 2022 13:10:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:54601) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1oXQT4-0001TX-En for bug-gnu-emacs@gnu.org; Sun, 11 Sep 2022 13:10:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1oXQT4-000671-3M for bug-gnu-emacs@gnu.org; Sun, 11 Sep 2022 13:10:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: =?UTF-8?Q?K=C3=A9vin?= Le Gouguec Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 11 Sep 2022 17:10:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 57733 X-GNU-PR-Package: emacs Original-Received: via spool by 57733-submit@debbugs.gnu.org id=B57733.166291615423437 (code B ref 57733); Sun, 11 Sep 2022 17:10:02 +0000 Original-Received: (at 57733) by debbugs.gnu.org; 11 Sep 2022 17:09:14 +0000 Original-Received: from localhost ([127.0.0.1]:43300 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oXQSI-00065x-3J for submit@debbugs.gnu.org; Sun, 11 Sep 2022 13:09:14 -0400 Original-Received: from mail-wm1-f54.google.com ([209.85.128.54]:55924) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1oXQSG-00065j-Im for 57733@debbugs.gnu.org; Sun, 11 Sep 2022 13:09:13 -0400 Original-Received: by mail-wm1-f54.google.com with SMTP id d5so5591317wms.5 for <57733@debbugs.gnu.org>; Sun, 11 Sep 2022 10:09:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:from:to:cc:subject:date; bh=FV3RLRlGqloIPJ+d9tcW8loGdwS8+vaONJg0c6e1Hqc=; b=qMILcGnpHEBKjebGLq3QEw9d6YRyTtOaSH/IJGLEI6AKfLaGY2E6npXo5GqXJ+/+0r KFlhpPgeTeBYPFdujKKUIKCj+a3y+6nh9AGFWUww8Q+9P3S1ppXlKKUozLzo7qalQgt0 /Ss1VWSlj+eZus8KnX3c2DoDSpm94gzXNgT8uzqMIB8vMElrDnrJHQT5iP9ngi10BXHl jwxWveqABp8pqdCrPg/aEoeYVC8OmbxfFhTUnrCrPKucEeMpcv1Itgco6cCI+w3PVeuH S8yUFeW3lC2rPvzR6JfemUCqcmd9ZyBdsOn+Zyd1rI0j0H0ctQ+XLDj/4vz+eTEXknM8 /lnw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:user-agent:message-id:date:references:in-reply-to :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date; bh=FV3RLRlGqloIPJ+d9tcW8loGdwS8+vaONJg0c6e1Hqc=; b=L1Jo88CtVgVvXEjHjLiDzfo556jDBwdwrvmHNR9USYHs7Zn4VpgzMDfr+Vv2/PBsVe EpEu6tyYOLllAPmsGEJRv3kQZtqTzPWUMVMITWZRqCrTxdB/mwQ3nqgY+wvtUt+B96bD qOF0MtXJlUuQUoo25aDDztvnUt2bRPb4a8t7ITnA6FaBdAjitB4XL7TW5Bt2pfgj2hhf 1ceR98PynV6loVVPn0/e2MBourSoLcdphfF0tPReiupgbMOYafhU8zm30LPG6wTDe6k3 2SUT8KWhJlUg4h/iWtd7gF80+PqyadA5ApsaubA60OUrZTIhlfBz978I6MpLCWPACW+r Lf1Q== X-Gm-Message-State: ACgBeo0rOEkq6HsnvFSkuxjJVCVg96NnVxZ7+iZY07+g5JlmQFL8RDRm HbBFKOKVZViNk6KQ04WXOCabhWcgQRg= X-Google-Smtp-Source: AA6agR7vVXtZMpRVfpJUTEGbJjFnD33j/EzdTz2WqhGEi4ZNhbTu5aODcx7fYPOgLyxV9ZRXJA5spg== X-Received: by 2002:a05:600c:1d14:b0:3a5:e8ba:f394 with SMTP id l20-20020a05600c1d1400b003a5e8baf394mr11402646wms.137.1662916146135; Sun, 11 Sep 2022 10:09:06 -0700 (PDT) Original-Received: from amdahl30 ([2a01:e0a:253:fe0:2ef0:5dff:fed2:7b49]) by smtp.gmail.com with ESMTPSA id 11-20020a05600c020b00b003b4868eb6bbsm1221783wmi.23.2022.09.11.10.09.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 11 Sep 2022 10:09:05 -0700 (PDT) In-Reply-To: <87illtdgxe.fsf@gnus.org> (Lars Ingebrigtsen's message of "Sun, 11 Sep 2022 18:17:17 +0200") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list 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-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:242189 Archived-At: --=-=-= Content-Type: text/plain Lars Ingebrigtsen writes: > Perhaps a simpler solution would to just be to use > `save-restriction'+`narrow-to-region' first and then don't use an END in > the search? That makes a lot of sense, thanks; new patch attached. --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0001-Restrict-replace-in-region-to-the-bounds-defined-by-.patch >From b5cc63cfd89f695067e9bb3a0135aded615557f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Le=20Gouguec?= Date: Sun, 11 Sep 2022 16:15:38 +0200 Subject: [PATCH] Restrict replace-*-in-region to the bounds defined by caller And call the corresponding *search-forward functions without giving them a bound. The previous code can lead to two surprising outcomes: (1) when REPLACEMENT is much larger than the original text, point might move beyond END, which causes the search functions to error out; (2) when REPLACEMENT is much shorter than the original text, the search functions might find matches that the user did not intend, as they were beyond END before the replacements happened. Fixes bug#57733. * lisp/subr.el (replace-string-in-region, replace-regexp-in-region): Narrow to region before iterating over matches, instead of giving a bound to the search functions. * test/lisp/subr-tests.el (test-replace-string-in-region): Add regression tests. --- lisp/subr.el | 38 +++++++++++++++++++++----------------- test/lisp/subr-tests.el | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 51 insertions(+), 19 deletions(-) diff --git a/lisp/subr.el b/lisp/subr.el index 686189e69b..8769fec2b9 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -4219,15 +4219,17 @@ replace-string-in-region (error "End after end of buffer")) (setq end (point-max))) (save-excursion - (let ((matches 0) - (case-fold-search nil)) - (goto-char start) - (while (search-forward string end t) - (delete-region (match-beginning 0) (match-end 0)) - (insert replacement) - (setq matches (1+ matches))) - (and (not (zerop matches)) - matches)))) + (goto-char start) + (save-restriction + (narrow-to-region start end) + (let ((matches 0) + (case-fold-search nil)) + (while (search-forward string nil t) + (delete-region (match-beginning 0) (match-end 0)) + (insert replacement) + (setq matches (1+ matches))) + (and (not (zerop matches)) + matches))))) (defun replace-regexp-in-region (regexp replacement &optional start end) "Replace REGEXP with REPLACEMENT in the region from START to END. @@ -4254,14 +4256,16 @@ replace-regexp-in-region (error "End after end of buffer")) (setq end (point-max))) (save-excursion - (let ((matches 0) - (case-fold-search nil)) - (goto-char start) - (while (re-search-forward regexp end t) - (replace-match replacement t) - (setq matches (1+ matches))) - (and (not (zerop matches)) - matches)))) + (goto-char start) + (save-restriction + (narrow-to-region start end) + (let ((matches 0) + (case-fold-search nil)) + (while (re-search-forward regexp nil t) + (replace-match replacement t) + (setq matches (1+ matches))) + (and (not (zerop matches)) + matches))))) (defun yank-handle-font-lock-face-property (face start end) "If `font-lock-defaults' is nil, apply FACE as a `face' property. diff --git a/test/lisp/subr-tests.el b/test/lisp/subr-tests.el index 4310b7291a..3011713210 100644 --- a/test/lisp/subr-tests.el +++ b/test/lisp/subr-tests.el @@ -968,7 +968,21 @@ test-replace-string-in-region (insert "Foo bar zot foobar") (should (= (replace-string-in-region "Foo" "new" (point-min)) 1)) - (should (equal (buffer-string) "new bar zot foobar")))) + (should (equal (buffer-string) "new bar zot foobar"))) + + (with-temp-buffer + (insert "foo bar baz") + (should (= (replace-string-in-region "ba" "quux corge grault" (point-min)) + 2)) + (should (equal (buffer-string) + "foo quux corge graultr quux corge graultz"))) + + (with-temp-buffer + (insert "foo bar bar") + (should (= (replace-string-in-region " bar" "" (point-min) 8) + 1)) + (should (equal (buffer-string) + "foo bar")))) (ert-deftest test-replace-regexp-in-region () (with-temp-buffer @@ -991,7 +1005,21 @@ test-replace-regexp-in-region (insert "Foo bar zot foobar") (should (= (replace-regexp-in-region "Fo+" "new" (point-min)) 1)) - (should (equal (buffer-string) "new bar zot foobar")))) + (should (equal (buffer-string) "new bar zot foobar"))) + + (with-temp-buffer + (insert "foo bar baz") + (should (= (replace-regexp-in-region "ba." "quux corge grault" (point-min)) + 2)) + (should (equal (buffer-string) + "foo quux corge grault quux corge grault"))) + + (with-temp-buffer + (insert "foo bar bar") + (should (= (replace-regexp-in-region " bar" "" (point-min) 8) + 1)) + (should (equal (buffer-string) + "foo bar")))) (ert-deftest test-with-existing-directory () (let ((dir (make-temp-name "/tmp/not-exist-"))) -- 2.37.3 --=-=-=--