From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#60467: 30.0.50; primitive-undo: Changes to be undone by function different from announced Date: Mon, 09 Jan 2023 01:03:43 -0500 Message-ID: References: <87o7ri74qv.fsf@localhost> <9bc9c69ac20a37ded741@heytings.org> <9bc9c69ac282c0148962@heytings.org> <87h6x9mgdv.fsf@localhost> Reply-To: Stefan Monnier Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="39838"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: 60467@debbugs.gnu.org, Gregory Heytings , Ihor Radchenko To: Alan Mackenzie Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Jan 09 07:04:18 2023 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 1pElGb-000A9N-9W for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 09 Jan 2023 07:04:17 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pElGQ-0002Hn-5d; Mon, 09 Jan 2023 01:04:06 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pElGM-0002F7-CY for bug-gnu-emacs@gnu.org; Mon, 09 Jan 2023 01:04:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pElGM-0004mU-3I for bug-gnu-emacs@gnu.org; Mon, 09 Jan 2023 01:04:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pElGL-0001MG-UC for bug-gnu-emacs@gnu.org; Mon, 09 Jan 2023 01:04:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 09 Jan 2023 06:04:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 60467 X-GNU-PR-Package: emacs Original-Received: via spool by 60467-submit@debbugs.gnu.org id=B60467.16732442345204 (code B ref 60467); Mon, 09 Jan 2023 06:04:01 +0000 Original-Received: (at 60467) by debbugs.gnu.org; 9 Jan 2023 06:03:54 +0000 Original-Received: from localhost ([127.0.0.1]:35580 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pElGD-0001Ls-Tn for submit@debbugs.gnu.org; Mon, 09 Jan 2023 01:03:54 -0500 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:34297) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pElGC-0001LZ-DV for 60467@debbugs.gnu.org; Mon, 09 Jan 2023 01:03:53 -0500 Original-Received: from pmg2.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 5D6B68058E; Mon, 9 Jan 2023 01:03:46 -0500 (EST) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 980A7806D4; Mon, 9 Jan 2023 01:03:44 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1673244224; bh=KA9z3QbK4EU6PepwGzPWp2uugnnAG9yr90mtC0QZe2s=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=Odn2SUG4+hvBTwV5WzF9kT82Xe+HgXWsFVY2fFLgvZ+JQnJOae4nE5XEtSWuiEzPU ohv1xVrp1Xyj93/asppmaVO4QLEfUzoBTLYEb88juLKxTAMAi8O6x3pBu6DhzWQU3B 0gTPdyz5kBF90yypCdPjVMkPSejnz+tTDw+b6g3E/JE+BTLsZZrYuMiPgvh4H7iE3S gYxAw4EH+08TqZJP6Ci0z8Qdlc7/TTpiSuxa3DhJtlWb9wiKVLGReYVaMcKYml53QU 5WZ57jEYbXzrT8+9ANGrPzZUpr5LW2yGvGH0MqzayTdMJhanGaBQZx5tFgUUiM/yTh TTe9cOTVZ3NBQ== Original-Received: from pastel (unknown [45.72.200.228]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 469E01201C2; Mon, 9 Jan 2023 01:03:44 -0500 (EST) In-Reply-To: (Alan Mackenzie's message of "Sun, 8 Jan 2023 15:43:36 +0000") 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-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:252986 Archived-At: >> Alan? Do you remember why you did that? > I'm afraid not. On 2018-04-01, I noted down that timestamps get "caught > up inside the undo--wrap-and-run-primitive-undo entry.". But I neglected > to be more specific, and can no longer remember exactly why. > >> What would go wrong if we applied a patch like the one below? > > I don't know. Possibly nothing. Maybe the problem that that code was > meant to solve was also solved by some other code. Thanks. So maybe we should strip timestamps on the release branch (just to be on the safe side) and keep them on `master` (since it's likely that we don't need to filter them out, which gives simpler code and might even be arguably more correct). On `emacs-29` the patch I suggest is: diff --git a/lisp/subr.el b/lisp/subr.el index 62f72734e14..34dd847e9d5 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -4946,13 +4946,13 @@ combine-change-calls-1 (progn (while (and (not (eq (cdr ptr) old-bul)) ;; In case garbage collection has removed OLD-BUL. - (cdr ptr) - ;; Don't include a timestamp entry. - (not (and (consp (cdr ptr)) - (consp (cadr ptr)) - (eq (caadr ptr) t) - (setq old-bul (cdr ptr))))) - (setq ptr (cdr ptr))) + (cdr ptr)) + (if (and (consp (cdr ptr)) + (consp (cadr ptr)) + (eq (caadr ptr) t)) + ;; Don't include a timestamp entry. + (setcdr ptr (cddr ptr)) + (setq ptr (cdr ptr)))) (unless (cdr ptr) (message "combine-change-calls: buffer-undo-list broken")) (setcdr ptr nil) I think it's the simplest&safest change to the code that fixes this bug. [ Of course, it should be accompanied by Gregory's tests. ] Then on master I suggest: diff --git a/lisp/subr.el b/lisp/subr.el index d1d3c76caf8..9e50b1e7f91 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -4966,21 +4966,20 @@ combine-change-calls-1 beg (marker-position end-marker) #'undo--wrap-and-run-primitive-undo - beg (marker-position end-marker) buffer-undo-list)) + beg (marker-position end-marker) + ;; We will truncate this list by side-effect below. + buffer-undo-list)) (ptr buffer-undo-list)) (if (not (eq buffer-undo-list old-bul)) (progn (while (and (not (eq (cdr ptr) old-bul)) ;; In case garbage collection has removed OLD-BUL. - (cdr ptr) - ;; Don't include a timestamp entry. - (not (and (consp (cdr ptr)) - (consp (cadr ptr)) - (eq (caadr ptr) t) - (setq old-bul (cdr ptr))))) + (or (cdr ptr) + (progn + (message "combine-change-calls: buffer-undo-list broken") + nil))) (setq ptr (cdr ptr))) - (unless (cdr ptr) - (message "combine-change-calls: buffer-undo-list broken")) + ;; Truncate the list that's in the `apply' entry. (setcdr ptr nil) (push ap-elt buffer-undo-list) (setcdr buffer-undo-list old-bul))))) We could also use Gregory's code on `master`. Obviously Gregory prefers it, tho it's marginally less efficient since it creates a new list instead of re-using the list elements already present. Stefan