From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: "Phillip Lord" Newsgroups: gmane.emacs.bugs Subject: bug#23785: Emacs 25: 'Undo' overdoes things. Date: Sun, 19 Jun 2016 23:45:44 +0100 Message-ID: References: <20160617150245.GB3316@acm.fritz.box> <83r3bvbuu1.fsf@gnu.org> <20160617174535.GD3316@acm.fritz.box> <83oa6zbmvd.fsf@gnu.org> <87ziqjwkrb.fsf@russet.org.uk> <83eg7vaq3z.fsf@gnu.org> <83bn2y9v80.fsf@gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed;boundary="----=_20160619234543_99489" X-Trace: ger.gmane.org 1466376388 23559 80.91.229.3 (19 Jun 2016 22:46:28 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 19 Jun 2016 22:46:28 +0000 (UTC) Cc: acm@muc.de, 23785@debbugs.gnu.org, phillip.lord@russet.org.uk To: "Stefan Monnier" Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Jun 20 00:46:17 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1bElTq-0008Fd-A6 for geb-bug-gnu-emacs@m.gmane.org; Mon, 20 Jun 2016 00:46:14 +0200 Original-Received: from localhost ([::1]:40343 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bElTp-0002ex-MK for geb-bug-gnu-emacs@m.gmane.org; Sun, 19 Jun 2016 18:46:13 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:51531) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bElTj-0002eb-03 for bug-gnu-emacs@gnu.org; Sun, 19 Jun 2016 18:46:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bElTe-0004c5-QL for bug-gnu-emacs@gnu.org; Sun, 19 Jun 2016 18:46:05 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:34435) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bElTe-0004c1-IL for bug-gnu-emacs@gnu.org; Sun, 19 Jun 2016 18:46:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1bElTe-0007ko-9Q for bug-gnu-emacs@gnu.org; Sun, 19 Jun 2016 18:46:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: "Phillip Lord" Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 19 Jun 2016 22:46:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 23785 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 23785-submit@debbugs.gnu.org id=B23785.146637635329792 (code B ref 23785); Sun, 19 Jun 2016 22:46:02 +0000 Original-Received: (at 23785) by debbugs.gnu.org; 19 Jun 2016 22:45:53 +0000 Original-Received: from localhost ([127.0.0.1]:46772 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bElTU-0007kS-MO for submit@debbugs.gnu.org; Sun, 19 Jun 2016 18:45:52 -0400 Original-Received: from cloud103.planethippo.com ([31.216.48.48]:50909) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bElTS-0007kC-Mg for 23785@debbugs.gnu.org; Sun, 19 Jun 2016 18:45:51 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=russet.org.uk; s=default; h=Content-Type:MIME-Version:Cc:To:From:Subject: Date:References:In-Reply-To:Message-ID; bh=9m1kPH+4Vt7lTWHABOWCOtZsUk5RD3TNjIfDJwy4dvg=; b=IjrpU0Xk7FSpN9tlMoJs54beot QKJUUUwLBEQihj916Bv7eTwuNe0N0WD0FB/VehPA4daGGF6HHxomvD0QQmrhcmnE4Dhr3GpaezYWc wmCOth6IayZxWLsPxqgfFzAhMAMaauXLuDNoXZdaUCU8T4NPhOV2yqjzjj5EUGf2sb5kKczKgbs7Y kOCEFv3bbKoaH9c2ASjeQ9FpahDPzmkCDzv2VwZ39KWnKcge3VJBjp/pBTj7cY3rVch/CwClOYfM1 ET/A7ELmZ+JCMrP1cSapMsVXk4dahQYEwsYM45Sx3GtzklhPfjZ9Ktgwe3FkDMcai27p9XUNK+aMk 79qg0G9A==; Original-Received: from [127.0.0.1] (port=40300 helo=cloud103.planethippo.com) by cloud103.planethippo.com with esmtpa (Exim 4.86_1) (envelope-from ) id 1bElTL-000Rqz-Vm; Sun, 19 Jun 2016 23:45:44 +0100 Original-Received: from 77.98.219.118 ([77.98.219.118]) (SquirrelMail authenticated user phillip.lord@russet.org.uk) by cloud103.planethippo.com with HTTP; Sun, 19 Jun 2016 23:45:44 +0100 In-Reply-To: User-Agent: SquirrelMail/1.5.2 [SVN] X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - cloud103.planethippo.com X-AntiAbuse: Original Domain - debbugs.gnu.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - russet.org.uk X-Get-Message-Sender-Via: cloud103.planethippo.com: authenticated_id: phillip.lord@russet.org.uk X-Authenticated-Sender: cloud103.planethippo.com: phillip.lord@russet.org.uk 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:119798 Archived-At: ------=_20160619234543_99489 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 8bit On Sat, June 18, 2016 8:52 pm, Stefan Monnier wrote: >> My concern is not with the behavior the proposed change intends to get >> us, the concern is with the unintended consequences of the change. At >> this late stage, I'd like to keep the risk of unintended consequences to >> the minimum, unless we want this change so badly we agree to extend the >> pretesting by another month or two. > > Yes, that's the other side of the coin, indeed. Attached are another two patches which also solve the issue. One adds an undo boundary specifically in revert-buffer. Works but is questionable in that revert-buffer is pluggable -- it can call any function, other than the default. The other fiddles with insert-file-contents and adds an undo. It is this function that has specialized handling for the undo list that is causing the problem. My patch in this case is questionable in that I have randomly pushed a call to undo-boundary near the end. It should probably be somewere better. Another possibility would be to have insert-file-contents call "undo-auto--undoable-change" -- this is the root cause of the problem. Changes are happening. Also attached is a test case which demonstrates the problem. So many possibilities! ------=_20160619234543_99489 Content-Type: text/x-patch; name="0001-Fix-missing-undo-boundary-after-revert-buffer.patch" Content-Transfer-Encoding: 8bit Content-Disposition: attachment; filename="0001-Fix-missing-undo-boundary-after-revert-buffer.patch" >From 4250d150cfce6cd461dc6475bac08d7587842e84 Mon Sep 17 00:00:00 2001 From: Phillip Lord Date: Sun, 19 Jun 2016 21:34:58 +0100 Subject: [PATCH] Fix missing undo-boundary after revert-buffer - lisp/files.el (revert-buffer): Ensure an undo-boundary after completion. Addresses Bug#23785 --- lisp/files.el | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lisp/files.el b/lisp/files.el index 1f97fa5..c5d2bab 100644 --- a/lisp/files.el +++ b/lisp/files.el @@ -5533,7 +5533,10 @@ revert-buffer (let ((revert-buffer-in-progress-p t) (revert-buffer-preserve-modes preserve-modes)) (funcall (or revert-buffer-function #'revert-buffer--default) - ignore-auto noconfirm))) + ignore-auto noconfirm) + ;; Ensure that we have a undo-boundary after reversion (see Bug + ;; #23785) + (undo-boundary))) (defun revert-buffer--default (ignore-auto noconfirm) "Default function for `revert-buffer'. -- 2.9.0 ------=_20160619234543_99489 Content-Type: text/x-patch; name="0001-Fix-missing-undo-boundary-on-revert.patch" Content-Transfer-Encoding: 8bit Content-Disposition: attachment; filename="0001-Fix-missing-undo-boundary-on-revert.patch" >From 1b2cd06f4b6f993052632e24c50209816314c004 Mon Sep 17 00:00:00 2001 From: Phillip Lord Date: Sun, 19 Jun 2016 23:33:22 +0100 Subject: [PATCH] Fix missing undo-boundary on revert But badly, because I have pushed the undo_boundary call into a fairly random place in a very long function. --- src/fileio.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/fileio.c b/src/fileio.c index b11f923..e01045e 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -4500,6 +4500,10 @@ by calling `format-decode', which see. */) current_buffer->newline_cache, PT - BEG, Z - PT - inserted); + /* Random push an undo boundary here because I don't know where else + to put this call */ + Fundo_boundary(); + if (read_quit) Fsignal (Qquit, Qnil); -- 2.9.0 ------=_20160619234543_99489 Content-Type: text/x-emacs-lisp; name="revert-buffer-tests.el" Content-Transfer-Encoding: 8bit Content-Disposition: attachment; filename="revert-buffer-tests.el" (require 'ert) (ert-deftest undo-after-revert-buffer () (should-not (unwind-protect (progn (find-file "simple-file.txt") (goto-char (point-min)) (insert "\n") (revert-buffer t t) (undo-auto--needs-boundary-p)) (kill-buffer (get-file-buffer "simple-file.txt"))))) ------=_20160619234543_99489 Content-Type: text/plain; name="simple-file.txt" Content-Transfer-Encoding: 8bit Content-Disposition: attachment; filename="simple-file.txt" line one ------=_20160619234543_99489--