From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#70077: An easier way to track buffer changes Date: Sat, 06 Apr 2024 11:43:36 +0300 Message-ID: <86frvy51af.fsf@gnu.org> References: Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="23568"; mail-complaints-to="usenet@ciao.gmane.io" Cc: acm@muc.de, yantar92@posteo.net, 70077@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Apr 06 10:44:13 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 1rt1em-0005tZ-AG for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 06 Apr 2024 10:44:12 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rt1eX-0005iT-BK; Sat, 06 Apr 2024 04:43:57 -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 1rt1eV-0005i5-UO for bug-gnu-emacs@gnu.org; Sat, 06 Apr 2024 04:43: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 1rt1eV-0003uL-MH for bug-gnu-emacs@gnu.org; Sat, 06 Apr 2024 04:43:55 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rt1eb-0005hL-NS for bug-gnu-emacs@gnu.org; Sat, 06 Apr 2024 04:44:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 06 Apr 2024 08:44: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.171239303421840 (code B ref 70077); Sat, 06 Apr 2024 08:44:01 +0000 Original-Received: (at 70077) by debbugs.gnu.org; 6 Apr 2024 08:43:54 +0000 Original-Received: from localhost ([127.0.0.1]:38206 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rt1eT-0005gC-UP for submit@debbugs.gnu.org; Sat, 06 Apr 2024 04:43:54 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:46238) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rt1eR-0005fL-3V for 70077@debbugs.gnu.org; Sat, 06 Apr 2024 04:43:53 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rt1eE-0003Cf-VI; Sat, 06 Apr 2024 04:43:38 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=Jiu2fiCBqPhYIp0Vdazq4VEmBhaG6Ji5mcRroBuSdU4=; b=AVkBQKTKRwk9 PxedgmLasWc0LvWLX4fERpm17IFHfja+/PxpgqzlI5IMV5J6LX74cj86hoD26lt0IQFpGYqgcTYXO 5NhXlUFqxZDNr/1CRssou3PLr/p2z9TNWYMIl23sg9bUSgPk2haHFHCfXFvpUk8RheJMPHKJS5IWi heREzv4f2m7SiVb2GhYLCF7ioXzMvY/HsbLfHoivLWaszYkP+x2FiG192uEOwgDa04CjQLkz9RVXo C+ig4v28n/tu4nLZXcalUPRYRFHRiM24+kXGeGoSsywO/KLb16fSP4me5z3DqDGEFchGNmU2O+uqd n21+0IsKUgs0IWra8KSgdg==; In-Reply-To: (bug-gnu-emacs@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:282759 Archived-At: > Cc: Alan Mackenzie , Ihor Radchenko > Date: Fri, 05 Apr 2024 18:12:55 -0400 > From: Stefan Monnier via "Bug reports for GNU Emacs, > the Swiss army knife of text editors" > > +;; This library is a layer of abstraction above `before-change-functions' > +;; and `after-change-functions' which takes care of accumulating changes > +;; until a time when its client finds it convenient to react to them. > +;; > +;; It provides an API that is easier to use correctly than our > +;; `*-change-functions` hooks. Problems that it claims to solve: Please redo all the quoting from `like this` to one of the two styles we use in our documentation. > +(unless (fboundp 'funcall-later) > + (defun funcall-later (&rest args) > + ;; FIXME: Not sure if `run-with-timer' preserves ordering between > + ;; different calls with the same target time. > + (apply #'run-with-timer 0 nil args))) Both funcall-later and run-with-timer?? > + > +;;;; Internal types and variables. > + > +(cl-defstruct (track-changes--tracker > + (:noinline t) > + (:constructor nil) > + (:constructor track-changes--tracker ( signal state > + &optional > + nobefore immediate))) > + signal state nobefore immediate) > + > +(cl-defstruct (track-changes--state > + (:noinline t) > + (:constructor nil) > + (:constructor track-changes--state ())) > + "Object holding a description of a buffer state. > +BEG..END is the area that was changed and BEFORE is its previous content. > +If the current buffer currently holds the content of the next state, you 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 doc string should IMO include the form of the object, because without that BEG, END, BEFORE, and NEXT are "out of the blue", and it's entirely unclear what they allude to. Also, this doc string (and a few others) have very long lines, so please refill them. > +(defvar-local track-changes--trackers () > + "List of trackers currently registered in the current buffer.") ^^^^^^^^^^^^^^^^^^^^^ I think "in the buffer" is more accurate, since this is not limited to the current buffer. > +(defvar-local track-changes--disjoint-trackers () > + "List of trackers that want to react to disjoint changes. > +These trackers' are signaled every time track-changes notices ^ That apostrophe is redundant. > +(defvar-local track-changes--before-clean 'unset > + "If non-nil, the `track-changes--before-*' vars are old. > +More specifically it means they cover a part of the buffer relevant > +for the previous state. > +It can take two non-nil values: > +- `unset': Means that the vars cover some older state. > + This is what it is set right after creating a fresh new state. ^^^ "set to" > +(cl-defun track-changes-register ( signal &key nobefore disjoint immediate) > + "Register a new tracker and return a new tracker ID. Please mention SIGNAL in the first line of the doc string. > +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 change. ^^ "we"? > +By default SIGNAL is called as soon as convenient after a change, which is ^^^^^^^^^^^^^^^^^^^^^ "as soon as it's convenient", I presume? > +usually right after the end of the current command. This should explicitly reference funcall-later, so that users could understand what "as soon as convenient" means. > +If optional argument DISJOINT is non-nil, SIGNAL is called every time we are > +about to combine changes from \"distant\" parts of the buffer. ^^ "we" again? > +In order to prevent the upcoming change from being combined with the previous > +changes, SIGNAL needs to call `track-changes-fetch' before it returns." This seems to contradict what the doc string says previously: that SIGNAL should NOT call track-changes-fetch. > +(defun track-changes-fetch (id func) > + "Fetch the pending changes. The first line of a doc string should mention the arguments. > +ID is the tracker ID returned by a previous `track-changes-register'. > +FUNC is a function. It is called with 3 arguments (BEGIN END BEFORE) > +where BEGIN..END delimit the region that was changed since the last > +time `track-changes-fetch' was called and BEFORE is a string containing > +the previous content of that region (or just its length as an integer > +If the tracker ID was registered with the `nobefore' option). ^^ "if", lower-case. > +If some error caused us to miss some changes, then BEFORE will be the ^^ "we" again? > +If no changes occurred since the last time, FUNC is not called and > +we return nil, otherwise we return the value returned by FUNC, ^^ And again. > + ;; The states are disconnected from the latest state because > + ;; we got out of sync! > + (cl-assert (eq (track-changes--state-before (car states)) 'error)) This seem to mean Emacs will signal an error in this case, not pass 'error' in BEFORE? > +(defun track-changes--clean-state () > + (cond > + ((null track-changes--state) > + (cl-assert track-changes--before-clean) > + (cl-assert (null track-changes--buffer-size)) > + ;; No state has been created yet. Do it now. > + (setq track-changes--buffer-size (buffer-size)) > + (when track-changes--before-no > + (setq track-changes--before-string (buffer-size))) > + (setq track-changes--state (track-changes--state))) > + (track-changes--before-clean nil) > + (t > + (cl-assert (<= (track-changes--state-beg track-changes--state) > + (track-changes--state-end track-changes--state))) > + (let ((actual-beg (track-changes--state-beg track-changes--state)) > + (actual-end (track-changes--state-end track-changes--state))) > + (if track-changes--before-no > + (progn > + (cl-assert (integerp track-changes--before-string)) > + (setf (track-changes--state-before track-changes--state) > + (- track-changes--before-string > + (- (buffer-size) (- actual-end actual-beg)))) > + (setq track-changes--before-string (buffer-size))) > + (cl-assert (<= track-changes--before-beg > + actual-beg actual-end > + track-changes--before-end)) > + (cl-assert (null (track-changes--state-before track-changes--state))) Are all those assertions a good idea in this function? I can envision using it as a cleanup, in which case assertions will not be appreciated. > +(defvar track-changes--disjoint-threshold 100 > + "Distance below which changes are not considered disjoint.") This should tell in what units the distance is measured. > +;;;; Extra candidates for the API. > + > +;; This could be a good alternative to using a temp-buffer like I used in ^^^^^^ "I"? > +;; Eglot, since presumably we've just been changing this very area of the > +;; buffer, so the gap should be ready nearby, > +;; It may seem silly to go back to the previous state, since we could have > +;; used `before-change-functions' to run FUNC right then when we were in > +;; that state. The advantage is that with track-changes we get to decide > +;; retroactively which state is the one for which we want to call FUNC and > +;; which BEG..END to use: when that state was current we may have known > +;; then that it would be "the one" but we didn't know what BEG and END > +;; should be because those depend on the changes that came afterwards. Suggest to reword (or remove) this comment, as it sounds like development-time notes. > +(defun diff--track-changes-function (beg end _before) > + (with-demoted-errors "%S" Why did you need with-demoted-errors here? Last, but not least: this needs suitable changes in NEWS and ELisp manual. Thanks.