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#70077: An easier way to track buffer changes Date: Mon, 08 Apr 2024 12:06:12 -0400 Message-ID: References: <87edbhnu53.fsf@localhost> Reply-To: Stefan Monnier 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="29146"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: Alan Mackenzie , 70077@debbugs.gnu.org To: Ihor Radchenko Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Apr 08 18:07:12 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 1rtrWa-0007LZ-4K for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 08 Apr 2024 18:07:12 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rtrWK-0003RX-RE; Mon, 08 Apr 2024 12:06:56 -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 1rtrWJ-0003RP-HO for bug-gnu-emacs@gnu.org; Mon, 08 Apr 2024 12:06:55 -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 1rtrWJ-0004Lp-97 for bug-gnu-emacs@gnu.org; Mon, 08 Apr 2024 12:06:55 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rtrWQ-00014X-K6 for bug-gnu-emacs@gnu.org; Mon, 08 Apr 2024 12:07:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 08 Apr 2024 16:07:02 +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.17125923943976 (code B ref 70077); Mon, 08 Apr 2024 16:07:02 +0000 Original-Received: (at 70077) by debbugs.gnu.org; 8 Apr 2024 16:06:34 +0000 Original-Received: from localhost ([127.0.0.1]:47481 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rtrVw-00011u-Ur for submit@debbugs.gnu.org; Mon, 08 Apr 2024 12:06:34 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:18304) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rtrVs-00010N-U2 for 70077@debbugs.gnu.org; Mon, 08 Apr 2024 12:06:31 -0400 Original-Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 6848B1000DD; Mon, 8 Apr 2024 12:06:15 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1712592374; bh=XrZIALznMd4ptZF6RpaujK5ZqFprQP2rWB3aztRKi80=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=kC0NMLAMeYZGhTChUwFcp5x9Ibeo4+mxGxFvYBVQ5ElLIbzdxYNDMpIiCDvceaC2H M51ZAIsniYD4DOgFyYErIva77A/R2jxmYViURtQ3F9pvlj5JZ1pa8/FsJK7wkemej3 wqnOFjCd0ICN2epBhOVNCYHvajkhsGObOhBL7AyjP+7oYnAF08CMcunUXw2GrJcUMk FXcHmkLJ/umCcEeyic5+7qBT7kmj/aeeGNPyMSedENmv4jy3nokspfFmtNJHwB+D4c ZZYPF+vDbSGhGjr2dkQQPzsL3k8mPjvQik0hwB71I0UoBsqx4nkh73czRirjDkxTrm 94aa1OM0GTwnQ== Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 36431100048; Mon, 8 Apr 2024 12:06:14 -0400 (EDT) Original-Received: from pastel (unknown [45.72.201.215]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 0FD1D120495; Mon, 8 Apr 2024 12:06:14 -0400 (EDT) In-Reply-To: <87edbhnu53.fsf@localhost> (Ihor Radchenko's message of "Sun, 07 Apr 2024 14:07: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:282946 Archived-At: >> + "Object holding a description of a buffer state. >> +BEG..END is the area that was changed and BEFORE is its previous conten= t. >> +If the current buffer currently holds the content of the next state, yo= u can get >> +the contents of the previous state with: >> + >> + (concat (buffer-substring (point-min) beg) >> + before >> + (buffer-substring end (point-max))) >> + >> +NEXT is the next state object (i.e. a more recent state). >> +If NEXT is nil it means it's most recent state and it may be incomplete >> +\(BEG/END/BEFORE may be nil), in which case those fields will take their >> +values from `track-changes--before-(beg|end|before)' when the next >> +state is create." > > 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. >> +(defvar-local track-changes--before-beg (point-min) >> + "Beginning position of the remembered \"before string\".") >> +(defvar-local track-changes--before-end (point-min) >> + "End position of the text replacing the \"before string\".") > Why (point-min)? It will make the values dependent on the buffer > narrowing that happens to be active when the library if first loaded. > Which cannot be right. The precise value should hopefully never matter, barring bugs. I changed them to 0. >> +(defvar-local track-changes--buffer-size nil >> + "Current size of the buffer, as far as this library knows. >> +This is used to try and detect cases where buffer modifications are \"l= ost\".") > Just looking at the buffer size may miss unregistered edits that do not > change the total size of the buffer. Although I do not know a better > measure. `buffer-chars-modified-tic' may lead to false-positives > (Bug#51766). Yup, hence the "try to". >> +(cl-defun track-changes-register ( signal &key nobefore disjoint immedi= ate) >> + "Register a new tracker and return a new tracker ID. >> +SIGNAL is a function that will be called with one argument (the tracker= ID) >> +after the current buffer is modified, so that we can react to the chang= e. >> + ... >> +If optional argument DISJOINT is non-nil, SIGNAL is called every time w= e are >> +about to combine changes from \"distant\" parts of the buffer. >> +This is needed when combining disjoint changes into one bigger change >> +is unacceptable, typically for performance reasons. >> +These calls are distinguished from normal calls by calling SIGNAL with >> +a second argument which is the distance between the upcoming change and >> +the previous changes. > > 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 call to signal disjoint changes. >> + (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 >> +(defun track-changes-fetch (id func) >> ... >> + (unless (equal track-changes--buffer-size (buffer-size)) >> + (track-changes--recover-from-error)) >> + (let ((beg nil) >> + (end nil) >> + (before t) >> + (lenbefore 0) >> + (states ())) >> + ;; Transfer the data from `track-changes--before-string' >> + ;; to the tracker's state object, if needed. >> + (track-changes--clean-state) > >> +(defun track-changes--recover-from-error () >> ... >> + (setq track-changes--state (track-changes--state))) > > This will create a dummy state with > > (beg (point-max)) > (end (point-min)) > > 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`. >> +(defun track-changes--in-revert (beg end before func) >> ... >> + (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 >> +(defun track-changes-tests--random-word () >> + (let ((chars ())) >> + (dotimes (_ (1+ (random 12))) >> + (push (+ ?A (random (1+ (- ?z ?A)))) chars)) >> + (apply #'string chars))) > > If you are using random values for edits, how can such test be > reproduced? Luck? > 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 Stefan diff --git a/test/lisp/emacs-lisp/track-changes-tests.el b/test/lisp/emacs-= lisp/track-changes-tests.el index cdccbe80299..eab9197030f 100644 --- a/test/lisp/emacs-lisp/track-changes-tests.el +++ b/test/lisp/emacs-lisp/track-changes-tests.el @@ -36,6 +36,11 @@ track-changes-tests--random-verbose (defun track-changes-tests--message (&rest args) (when track-changes-tests--random-verbose (apply #'message args))) =20 +(defvar track-changes-tests--random-seed + (let ((seed (number-to-string (random (expt 2 24))))) + (message "Random seed =3D %S" seed) + seed)) + (ert-deftest track-changes-tests--random () ;; Keep 2 buffers in sync with a third one as we make random ;; changes to that 3rd one. @@ -97,6 +102,8 @@ track-changes-tests--random (insert-into-buffer buf2) (should (equal (buffer-hash) (buffer-hash buf1))) (should (equal (buffer-hash) (buffer-hash buf2))) + (message "seeding with: %S" track-changes-tests--random-seed) + (random track-changes-tests--random-seed) (dotimes (_ 1000) (pcase (random 15) (0