From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Ihor Radchenko Newsgroups: gmane.emacs.bugs Subject: bug#70077: An easier way to track buffer changes Date: Tue, 09 Apr 2024 17:35:48 +0000 Message-ID: <87msq2phfv.fsf@localhost> References: <87edbhnu53.fsf@localhost> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="3648"; mail-complaints-to="usenet@ciao.gmane.io" Cc: Alan Mackenzie , 70077@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Tue Apr 09 19:36:15 2024 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 1ruFOH-0000ey-W1 for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 09 Apr 2024 19:36:14 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1ruFNz-0002jj-Ix; Tue, 09 Apr 2024 13:35:55 -0400 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 1ruFNx-0002jR-Qy for bug-gnu-emacs@gnu.org; Tue, 09 Apr 2024 13:35:53 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1ruFNx-00012B-J2 for bug-gnu-emacs@gnu.org; Tue, 09 Apr 2024 13:35:53 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1ruFO5-0004Yq-Lw for bug-gnu-emacs@gnu.org; Tue, 09 Apr 2024 13:36:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Ihor Radchenko Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 09 Apr 2024 17:36:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 70077 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 70077-submit@debbugs.gnu.org id=B70077.171268414917490 (code B ref 70077); Tue, 09 Apr 2024 17:36:01 +0000 Original-Received: (at 70077) by debbugs.gnu.org; 9 Apr 2024 17:35:49 +0000 Original-Received: from localhost ([127.0.0.1]:51434 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ruFNs-0004Y2-P2 for submit@debbugs.gnu.org; Tue, 09 Apr 2024 13:35:49 -0400 Original-Received: from mout01.posteo.de ([185.67.36.65]:49779) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ruFNo-0004X2-14 for 70077@debbugs.gnu.org; Tue, 09 Apr 2024 13:35:46 -0400 Original-Received: from submission (posteo.de [185.67.36.169]) by mout01.posteo.de (Postfix) with ESMTPS id 9F81C240029 for <70077@debbugs.gnu.org>; Tue, 9 Apr 2024 19:35:29 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1712684129; bh=qH+abJ0G73HKZonq144dnz6PZU/G+X23E+LMvqPI924=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version:Content-Type: Content-Transfer-Encoding:From; b=eE2a1OpjPbFtOxEmdLMLBjktMGKPd/TlmR2GPe1RIM3t9TUHSpeTf1BcBWdXnCFFF JhIQfHksvydOhKzARsvQIEvYWMWQQKEdxp7MQIj3D4YZ03yqLLtjW4YNezF9pP9z35 dbYdns5gxh4hdMBapZZmixMjioK/84mGJn+JEYdTGaWNfDKo8Ooys/fYo+FIPq0xFf sKs1z3fHje2W7JCiHp3emdbHg7S6F9RcVn45lcoFyfTBAULBJ9f6IZDWlbEajHiFz0 dyuYEsCSOYByEBVEdys1y8jij0+o+caLA8Uj1ZCK/BNsj3Ai1U4y/Y0TtFNhzpb6rZ fxxLqR5Xw6j8w== Original-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4VDY5D5rMsz9rxG; Tue, 9 Apr 2024 19:35:28 +0200 (CEST) In-Reply-To: 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:283003 Archived-At: Stefan Monnier writes: >> ... >> This docstring is a bit confusing. >> If a state object is not the most recent, how come=20 >> >>> + (concat (buffer-substring (point-min) beg) >>> + before >>> + (buffer-substring end (point-max))) >> >> produces the previous content? > > Because it says "If the current buffer currently holds the content of > the next state". > [ "current ... currently" wasn't my best moment, admittedly. ] > >> And if the state object is the most recent, "it may be incomplete"... >> So, when is it safe to use the above (concat ... ) call? > > You never want to use this call, it's only there to show in a concise > manner how BEG/END/BEFORE relate and what information they're supposed > to hold. My point is that it is hard to understand that docstring for someone reading it first. I did not mean that it does not make sense for someone who is already familiar with the rest of the code. I believe that it would help if you explain how to interpret the values when there is a chain of state1 -> next -> next -> nil. >> This is a bit confusing. The first paragraph says that SIGNAL is called >> with a single argument, but that it appears that two arguments may be >> passed. I'd rather tell the calling convention early in the docstring. > > The second convention is used only when DISJOINT is non-nil, which is > why it's described where we document the effect of DISJOINT. > An alternative could be to make the `disjoint` arg hold the function to c= all > to signal disjoint changes. Sure. My point is that the current formulation of the docstring might lead to errors unless one is very careful when reading it. >>> + (unless nobefore >>> + (setq track-changes--before-no nil) >>> + (add-hook 'before-change-functions #'track-changes--before nil t)) >>> + (add-hook 'after-change-functions #'track-changes--after nil t) >> Note that all the changes made in indirect buffers will be missed. >> See bug#60333. > > Yup. And misuses of `inhibit-modification-hooks` or > `with-silent-modifications`. =F0=9F=99=81 Indirect buffer is not at all a misuse. It is a perfectly valid use. What I implied in my comment is that it would be nice to implement something like what whitespace.el does in bug#60333. This point is important from the point of view of Org mode because Org mode uses a lot of indirect buffers both internally and as a recommended workflow (org-tree-to-indirect-buffer). >> ... >> such state will not pass (< beg end) assertion in >> `track-changes--clean-state' called in `track-changes-fetch' immediately >> after `track-changes--recover-from-error' > > I can't reproduce that. Do you have a recipe? > > AFAICT all the (< beg end) tests in `track-changes--clean-state` are > conditional on `track-changes--before-clean` being nil, whereas > `track-changes--recover-from-error` sets that var to `unset`. You are right. That sneaky (track-changes--before-clean nil) line is so sneaky that I'd like to see a comment highlighting that it exists ;) I simply missed it and thought that `track-changes--before-clean' only has two cond branches when I was reading the code. >>> + (atomic-change-group >>> + (goto-char end) >>> + (insert-before-markers before) >>> + (delete-region beg end) >> >> What happens if there are markers inside beg...end? > > During FUNC they're moved to BEG or END, and when we restore the > original state, well... the undo machinery has some support to restore > the markers where they were, but it's definitely not 100%. =F0=9F=99=81 Interesting. I did not know that markers are recovered by undo. I had very bad experience with marker preservation when working with other low-level APIs like `replace-match'. And previous Org mode commiters had the same issues with kill/yank. We even have a dedicated machinery in Org mode to save/restore markers in region. For example, see `org-save-markers-in-region' `org-reinstall-markers-in-region' (there are more in various places). May undo be trusted? Or may it be more reliable to deploy a more manual approach explicitly saving the markers and restoring them via unwind-protect? >> If you are using random values for edits, how can such test be >> reproduced? > > Luck? Such test is barely useful then, IMHO. >> Maybe first generate a random seed and then log it, so that the >> failing test can be repeated if necessary with seed assigned manually. > > Good idea. > But my attempt (see patch below) failed. > I'm not sure what I'm doing wrong, but > > make test/lisp/emacs-lisp/track-changes-tests \ > EMACS_EXTRAOPT=3D"--eval '(setq track-changes-tests--random-seed= \"15216888\")'" > > gives a different score each time. =F0=9F=99=81 May it be because you used different Emacs builds? Elisp manual 3.10 Random Numbers Sometimes you want the random number sequence to be repeatable. For example, when debugging a program whose behavior depends on the random number sequence, it is helpful to get the same behavior in each program run. To make the sequence repeat, execute =E2=80=98(random = "")=E2=80=99. This sets the seed to a constant value for your particular Emacs executable (though it may differ for other Emacs builds). You can use ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ other strings to choose various seed values. As an alternative, you can use `cl-make-random-state' + `cl-random'. They are less random AFAIU, but it is not a big deal in this particular use case. --=20 Ihor Radchenko // yantar92, Org mode contributor, Learn more about Org mode at . Support Org development at , or support my work at