From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Gregory Heytings Newsgroups: gmane.emacs.bugs Subject: bug#60467: 30.0.50; primitive-undo: Changes to be undone by function different from announced Date: Tue, 03 Jan 2023 15:05:29 +0000 Message-ID: References: <87o7ri74qv.fsf@localhost> <9bc9c69ac20a37ded741@heytings.org> <9bc9c69ac282c0148962@heytings.org> <87h6x9mgdv.fsf@localhost> <838rij23by.fsf@gnu.org> <83zgazznau.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; format=flowed; charset=us-ascii Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="16807"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 60467@debbugs.gnu.org, yantar92@posteo.net, monnier@iro.umontreal.ca To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Jan 03 16:06:15 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 1pCirm-000460-LJ for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 03 Jan 2023 16:06:14 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pCirc-00033M-Aw; Tue, 03 Jan 2023 10:06:04 -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 1pCira-00033A-9m for bug-gnu-emacs@gnu.org; Tue, 03 Jan 2023 10:06: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 1pCirZ-0006LW-VY for bug-gnu-emacs@gnu.org; Tue, 03 Jan 2023 10:06:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pCirZ-0008U6-Qw for bug-gnu-emacs@gnu.org; Tue, 03 Jan 2023 10:06:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Gregory Heytings Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 03 Jan 2023 15:06: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.167275833332567 (code B ref 60467); Tue, 03 Jan 2023 15:06:01 +0000 Original-Received: (at 60467) by debbugs.gnu.org; 3 Jan 2023 15:05:33 +0000 Original-Received: from localhost ([127.0.0.1]:46479 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pCir7-0008TB-3O for submit@debbugs.gnu.org; Tue, 03 Jan 2023 10:05:33 -0500 Original-Received: from heytings.org ([95.142.160.155]:57252) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pCir4-0008T0-S4 for 60467@debbugs.gnu.org; Tue, 03 Jan 2023 10:05:32 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=heytings.org; s=20220101; t=1672758329; bh=bcbaaINL7NgkTtowVAEbdhITYy2/VIyggNfoJikdiWY=; h=Date:From:To:cc:Subject:In-Reply-To:Message-ID:References:From; b=zm3mU5ajCEpNhbipfcAsPBrCjD162LL+oeLf8MocSSKypuhKoYvb4/OE1YJPYSpGq z+9PSUKs6ivyuPlqp2/KCHDkMFqmRGMIwvXT+PFMzHRxQfl7fWNUWJ2B3vjFyPj/Jx wOtUj1tt4PC25tpqcDbQXo/fHohOr6/DtYFeSNaIAv6obBI1J3Me0b6iwbMIXbxGLQ YdJWXCb/DS9jsfLrXDYkOHOOP6NAyv4TeuJIEFMx7ozT35e4LqkS//Y2TW3/9qjiD6 EvnVaYAH3Igx9Rz5y7SyZNUiCmQFOl32PYNPoZB1CBghXO4p8KjJ/YybvRamI7GGdf sLFqlkw7R2iZA== In-Reply-To: <83zgazznau.fsf@gnu.org> 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:252411 Archived-At: >> - ;; In case garbage collection has removed OLD-BUL. >> - (cdr ptr) > > Why should we toss this precaution? > I don't even understand what this is supposed to do. By definition, garbage collection cannot collect something that is still referred to. >> - (unless (cdr ptr) >> - (message "combine-change-calls: buffer-undo-list broken")) > > And this one? > This one is just plain wrong. It assumes that buffer-undo-list is non-nil initially. >> + ;; If buffer-undo-list is neither t (in which case undo >> + ;; information is not recorded) nor equal to buffer-undo-list >> + ;; before body was evaluated (in which case evaluating body >> + ;; did not add items to buffer-undo-list) ... >> + (when (and (not (eq buffer-undo-list t)) >> + (not (eq buffer-undo-list old-bul))) >> + (let ((ptr buffer-undo-list) body-undo-list) >> + ;; ... then loop over buffer-undo-list, until the head of >> + ;; buffer-undo-list before body was evaluated is found ... >> + (while (not (eq ptr old-bul)) >> + ;; ... and add the entries to body-undo-list, unless >> + ;; they are of the form (t . ), which are >> + ;; entries that record buffer modification timestamps. >> + (unless (and (consp (car ptr)) >> + (eq (caar ptr) t)) >> + (push (car ptr) body-undo-list)) >> + (setq ptr (cdr ptr))) >> + (setq body-undo-list (nreverse body-undo-list)) >> + ;; Add an (apply ...) entry to buffer-undo-list, using >> + ;; body-undo-list ... >> + (push (list 'apply >> + (- end end-marker) >> + beg >> + (marker-position end-marker) >> + #'undo--wrap-and-run-primitive-undo >> + beg (marker-position end-marker) >> + body-undo-list) >> + buffer-undo-list) >> + ;; ... and set the cdr of buffer-undo-list to >> + ;; buffer-undo-list before body was evaluated. >> + (setcdr buffer-undo-list old-bul))) > > So basically you are saying that the current code stops too early and > doesn't collect all the undo entries that need to be combined, because > timestamp entries get in the way, is that right? > Yes. The current code apparently meant to skip these entries, but it does not work at all. Replacing a broken code that does not work with a clean(er) code that does work seems the right thing.