* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving [not found] ` <20220507100605.B7CA7C051FF@vcs2.savannah.gnu.org> @ 2022-05-07 16:06 ` Stefan Monnier 2022-05-07 16:10 ` Eli Zaretskii 2022-05-07 16:49 ` Lars Ingebrigtsen 2022-05-09 12:37 ` Lars Ingebrigtsen 1 sibling, 2 replies; 25+ messages in thread From: Stefan Monnier @ 2022-05-07 16:06 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: emacs-devel > - (let ((modified (make-symbol "modified"))) > + (let ((modified (make-symbol "modified")) > + (tick (make-symbol "tick"))) > `(let* ((,modified (buffer-modified-p)) > + (,tick (buffer-modified-tick)) > (buffer-undo-list t) > (inhibit-read-only t) > (inhibit-modification-hooks t)) > (unwind-protect > (progn > ,@body) > + ;; We restore the buffer tick count, too, because otherwise > + ;; we'll trigger a new auto-save. > + (internal--set-buffer-modified-tick ,tick) > (unless ,modified > (restore-buffer-modified-p nil)))))) BTW, I wonder if the auto-save mechanism should use the CHARS_MODIFF ticks instead? Stefan ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-07 16:06 ` master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving Stefan Monnier @ 2022-05-07 16:10 ` Eli Zaretskii 2022-05-07 16:27 ` Stefan Monnier 2022-05-07 16:49 ` Lars Ingebrigtsen 1 sibling, 1 reply; 25+ messages in thread From: Eli Zaretskii @ 2022-05-07 16:10 UTC (permalink / raw) To: Stefan Monnier; +Cc: larsi, emacs-devel > From: Stefan Monnier <monnier@iro.umontreal.ca> > Cc: emacs-devel@gnu.org > Date: Sat, 07 May 2022 12:06:10 -0400 > > BTW, I wonder if the auto-save mechanism should use the CHARS_MODIFF > ticks instead? Why would we want to change how auto-save have worked for decades?? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-07 16:10 ` Eli Zaretskii @ 2022-05-07 16:27 ` Stefan Monnier 2022-05-07 16:40 ` Eli Zaretskii 0 siblings, 1 reply; 25+ messages in thread From: Stefan Monnier @ 2022-05-07 16:27 UTC (permalink / raw) To: Eli Zaretskii; +Cc: larsi, emacs-devel Eli Zaretskii [2022-05-07 19:10:08] wrote: >> BTW, I wonder if the auto-save mechanism should use the CHARS_MODIFF >> ticks instead? > Why would we want to change how auto-save have worked for decades?? For the same reason we just changed the way `with-silent-modifications` (and lots of other code which either used `with-silent-modifications` or used their own local copy of basically the same code)? Stefan ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-07 16:27 ` Stefan Monnier @ 2022-05-07 16:40 ` Eli Zaretskii 2022-05-07 17:21 ` Stefan Monnier 0 siblings, 1 reply; 25+ messages in thread From: Eli Zaretskii @ 2022-05-07 16:40 UTC (permalink / raw) To: Stefan Monnier; +Cc: larsi, emacs-devel > From: Stefan Monnier <monnier@iro.umontreal.ca> > Cc: larsi@gnus.org, emacs-devel@gnu.org > Date: Sat, 07 May 2022 12:27:04 -0400 > > Eli Zaretskii [2022-05-07 19:10:08] wrote: > >> BTW, I wonder if the auto-save mechanism should use the CHARS_MODIFF > >> ticks instead? > > Why would we want to change how auto-save have worked for decades?? > > For the same reason we just changed the way `with-silent-modifications` > (and lots of other code which either used `with-silent-modifications` > or used their own local copy of basically the same code)? Not good enough: auto-saving is a critical feature, and I object to changes that could make it less reliable, because it could mean users will lose their edits. By contrast, with-silent-modifications is just a convenience device, to avoid annoying users with redundant "changes" of the buffer. What practical problem do you want to solve by that change in auto-save? ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-07 16:40 ` Eli Zaretskii @ 2022-05-07 17:21 ` Stefan Monnier 2022-05-07 17:26 ` Eli Zaretskii 0 siblings, 1 reply; 25+ messages in thread From: Stefan Monnier @ 2022-05-07 17:21 UTC (permalink / raw) To: Eli Zaretskii; +Cc: larsi, emacs-devel Eli Zaretskii [2022-05-07 19:40:42] wrote: > Not good enough: auto-saving is a critical feature, and I object to > changes that could make it less reliable, because it could mean users > will lose their edits. By contrast, with-silent-modifications is just > a convenience device, to avoid annoying users with redundant "changes" > of the buffer. Your call. The change to `with-silent-modifications` can similarly break auto-save, of course. > What practical problem do you want to solve by that change in > auto-save? The motivation for the change makes me think that the change I propose might be closer to the ideal. Stefan ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-07 17:21 ` Stefan Monnier @ 2022-05-07 17:26 ` Eli Zaretskii 0 siblings, 0 replies; 25+ messages in thread From: Eli Zaretskii @ 2022-05-07 17:26 UTC (permalink / raw) To: Stefan Monnier; +Cc: larsi, emacs-devel > From: Stefan Monnier <monnier@iro.umontreal.ca> > Cc: larsi@gnus.org, emacs-devel@gnu.org > Date: Sat, 07 May 2022 13:21:40 -0400 > > Eli Zaretskii [2022-05-07 19:40:42] wrote: > > Not good enough: auto-saving is a critical feature, and I object to > > changes that could make it less reliable, because it could mean users > > will lose their edits. By contrast, with-silent-modifications is just > > a convenience device, to avoid annoying users with redundant "changes" > > of the buffer. > > Your call. The change to `with-silent-modifications` can similarly > break auto-save, of course. If it does, we will hear about it sooner or later. But code that uses with-silent-modifications is but a fraction of the code that changes buffers in some way. And when some code is inside with-silent-modifications, it is clearly marked, so its effects on auto-save are easily seen and understood. Thus, chances are relatively small that we allow some changes that should cause auto-save be hidden by with-silent-modifications. > > What practical problem do you want to solve by that change in > > auto-save? > > The motivation for the change makes me think that the change I propose > might be closer to the ideal. The ideal for auto-saving is to save any changes, and when in doubt, save anyway. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-07 16:06 ` master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving Stefan Monnier 2022-05-07 16:10 ` Eli Zaretskii @ 2022-05-07 16:49 ` Lars Ingebrigtsen 1 sibling, 0 replies; 25+ messages in thread From: Lars Ingebrigtsen @ 2022-05-07 16:49 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: > BTW, I wonder if the auto-save mechanism should use the CHARS_MODIFF > ticks instead? Some modes may legitimately want to count non-character changes as something that should trigger auto-save (I'm thinking of rich text modes where you apply a bold face to the text to signal that it should write out <bold>). So I think leaving this up to the modes (which is what we basically (tried to) do with with-silent-modifications) is the right thing. Probably. But I guess we could have a use-modiff-instead-of-char-modiff for these modes (there aren't very many). But as Eli says, it's difficult to say what the repercussions might be to code out there, and we certainly don't want anybody to lose any text, so leaving it as is is probably best. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving [not found] ` <20220507100605.B7CA7C051FF@vcs2.savannah.gnu.org> 2022-05-07 16:06 ` master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving Stefan Monnier @ 2022-05-09 12:37 ` Lars Ingebrigtsen 2022-05-09 13:22 ` Stefan Monnier 1 sibling, 1 reply; 25+ messages in thread From: Lars Ingebrigtsen @ 2022-05-09 12:37 UTC (permalink / raw) To: emacs-devel Lars Ingebrigtsen <larsi@gnus.org> writes: > * lisp/subr.el (with-silent-modifications): Use it to restore the > ticks (bug#11303). Hm, this had one unintended consequence, apparently -- in a mode I have that inserts image thumbnails (using with-silent-modifications when slapping a 'display property over some text), the buffer wasn't updated afterwards... So perhaps this needs a rethink, or (at least) documentation. But it's slightly unusual -- it's running very asynchronously, so there's no user input while it's fetching the images from the net. (Once I do anything in Emacs, it updates the window.) Is that why the effect isn't noticeable with normal font locking? Hm... -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-09 12:37 ` Lars Ingebrigtsen @ 2022-05-09 13:22 ` Stefan Monnier 2022-05-09 13:35 ` Lars Ingebrigtsen 0 siblings, 1 reply; 25+ messages in thread From: Stefan Monnier @ 2022-05-09 13:22 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: emacs-devel Lars Ingebrigtsen [2022-05-09 14:37:00] wrote: > Hm, this had one unintended consequence, apparently -- in a mode I have > that inserts image thumbnails (using with-silent-modifications when > slapping a 'display property over some text), the buffer wasn't updated > afterwards... So perhaps this needs a rethink, or (at least) > documentation. I'm not completely surprised, to be honest. The handling of MODIFFs is surprisingly delicate. Take a look at the code for `Frestore_buffer_modified_p` to get an idea. Setting MODIFF back to a previous value is something I would not recommend. Stefan ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-09 13:22 ` Stefan Monnier @ 2022-05-09 13:35 ` Lars Ingebrigtsen 2022-05-09 13:47 ` Lars Ingebrigtsen 0 siblings, 1 reply; 25+ messages in thread From: Lars Ingebrigtsen @ 2022-05-09 13:35 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: > I'm not completely surprised, to be honest. The handling of MODIFFs is > surprisingly delicate. Take a look at the code for > `Frestore_buffer_modified_p` to get an idea. Setting MODIFF back to > a previous value is something I would not recommend. Hm... /* Here we have a problem. SAVE_MODIFF is used here to encode buffer-modified-p (as SAVE_MODIFF<MODIFF) as well as recent-auto-save-p (as SAVE_MODIFF<auto_save_modified). So if we modify SAVE_MODIFF to affect one, we may affect the other as well. E.g. if FLAG is nil we need to set SAVE_MODIFF to MODIFF, but if SAVE_MODIFF<auto_save_modified that means we risk changing recent-auto-save-p from t to nil. Vice versa, if FLAG is non-nil and SAVE_MODIFF>=auto_save_modified we risk changing recent-auto-save-p from nil to t. */ SAVE_MODIFF = (NILP (flag) /* FIXME: This unavoidably sets recent-auto-save-p to nil. */ ? MODIFF /* Let's try to preserve recent-auto-save-p. */ : SAVE_MODIFF < MODIFF ? SAVE_MODIFF /* If SAVE_MODIFF == auto_save_modified == MODIFF, we can either decrease SAVE_MODIFF and auto_save_modified or increase MODIFF. */ : modiff_incr (&MODIFF)); (Ah, and FLAG isn't documented in the doc string, which perhaps it should be?) Well... would upping auto_save_modified to MODIFF in with-silent-modifications do the trick instead? (I mean, if recent-auto-save-p was already true when the macro was entered.) I.e., reverse which modiff we're twiddling here. Hm... -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-09 13:35 ` Lars Ingebrigtsen @ 2022-05-09 13:47 ` Lars Ingebrigtsen 2022-05-09 15:43 ` Stefan Monnier 0 siblings, 1 reply; 25+ messages in thread From: Lars Ingebrigtsen @ 2022-05-09 13:47 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Lars Ingebrigtsen <larsi@gnus.org> writes: > Well... would upping auto_save_modified to MODIFF in > with-silent-modifications do the trick instead? (I mean, if > recent-auto-save-p was already true when the macro was entered.) I.e., > reverse which modiff we're twiddling here. I.e., have a special value for FLAG that says "update BUF_AUTOSAVE_MODIFF to MODIFF" in restore-buffer-modified-p. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-09 13:47 ` Lars Ingebrigtsen @ 2022-05-09 15:43 ` Stefan Monnier 2022-05-09 16:05 ` Lars Ingebrigtsen 0 siblings, 1 reply; 25+ messages in thread From: Stefan Monnier @ 2022-05-09 15:43 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: emacs-devel Lars Ingebrigtsen [2022-05-09 15:47:43] wrote: > Lars Ingebrigtsen <larsi@gnus.org> writes: >> Well... would upping auto_save_modified to MODIFF in >> with-silent-modifications do the trick instead? (I mean, if >> recent-auto-save-p was already true when the macro was entered.) I.e., >> reverse which modiff we're twiddling here. > I.e., have a special value for FLAG that says "update > BUF_AUTOSAVE_MODIFF to MODIFF" in restore-buffer-modified-p. Maybe we could make `buffer-modified-p` return `autosaved` rather than `t` when the buffer is modified but auto-saved, and then have `restore-buffer-modified-p` accept such a value accordingly, yes. Stefan ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-09 15:43 ` Stefan Monnier @ 2022-05-09 16:05 ` Lars Ingebrigtsen 2022-05-09 16:30 ` Eli Zaretskii 2022-05-09 17:00 ` Lars Ingebrigtsen 0 siblings, 2 replies; 25+ messages in thread From: Lars Ingebrigtsen @ 2022-05-09 16:05 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel, Eli Zaretskii Stefan Monnier <monnier@iro.umontreal.ca> writes: > Maybe we could make `buffer-modified-p` return `autosaved` rather than > `t` when the buffer is modified but auto-saved, and then > have `restore-buffer-modified-p` accept such a value accordingly, yes. Unfortunately `buffer-modified-p' is documented to return t... Changing that feels a bit "eh". (Which is why we should always say non-nil.) I don't think adding an extra call to `recent-auto-save-p' to the macro would be that bad, but if people think altering the return value of `buffer-modified-p' is OK, we can do that. Eli, what do you think? -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-09 16:05 ` Lars Ingebrigtsen @ 2022-05-09 16:30 ` Eli Zaretskii 2022-05-09 16:45 ` Lars Ingebrigtsen 2022-05-09 17:00 ` Lars Ingebrigtsen 1 sibling, 1 reply; 25+ messages in thread From: Eli Zaretskii @ 2022-05-09 16:30 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: monnier, emacs-devel > From: Lars Ingebrigtsen <larsi@gnus.org> > Cc: emacs-devel@gnu.org, Eli Zaretskii <eliz@gnu.org> > Date: Mon, 09 May 2022 18:05:55 +0200 > > Stefan Monnier <monnier@iro.umontreal.ca> writes: > > > Maybe we could make `buffer-modified-p` return `autosaved` rather than > > `t` when the buffer is modified but auto-saved, and then > > have `restore-buffer-modified-p` accept such a value accordingly, yes. > > Unfortunately `buffer-modified-p' is documented to return t... Changing > that feels a bit "eh". (Which is why we should always say non-nil.) > > I don't think adding an extra call to `recent-auto-save-p' to the macro > would be that bad, but if people think altering the return value of > `buffer-modified-p' is OK, we can do that. Eli, what do you think? I don't see any Lisp comparing the return value to t, and only one place in C which might need some change due to this, so I think returning non-nil, non-t value from buffer-modified-p is a better way. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-09 16:30 ` Eli Zaretskii @ 2022-05-09 16:45 ` Lars Ingebrigtsen 2022-05-09 17:40 ` Lars Ingebrigtsen 0 siblings, 1 reply; 25+ messages in thread From: Lars Ingebrigtsen @ 2022-05-09 16:45 UTC (permalink / raw) To: Eli Zaretskii; +Cc: monnier, emacs-devel Eli Zaretskii <eliz@gnu.org> writes: > I don't see any Lisp comparing the return value to t, and only one > place in C which might need some change due to this, so I think > returning non-nil, non-t value from buffer-modified-p is a better way. Okidoke; I'll modify these functions (and the macro) as discussed, then. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-09 16:45 ` Lars Ingebrigtsen @ 2022-05-09 17:40 ` Lars Ingebrigtsen 2022-05-09 18:29 ` Stefan Monnier 0 siblings, 1 reply; 25+ messages in thread From: Lars Ingebrigtsen @ 2022-05-09 17:40 UTC (permalink / raw) To: Eli Zaretskii; +Cc: monnier, emacs-devel The following builds (from bootstrap), all the tests pass, and it seems to work. But I'd like some more eyeballs on it anyway. 😓 diff --git a/doc/lispref/buffers.texi b/doc/lispref/buffers.texi index d8cf3d7919..3e3b0bd9f0 100644 --- a/doc/lispref/buffers.texi +++ b/doc/lispref/buffers.texi @@ -541,10 +541,12 @@ Buffer Modification @ref{Text}. @defun buffer-modified-p &optional buffer -This function returns @code{t} if the buffer @var{buffer} has been modified -since it was last read in from a file or saved, or @code{nil} -otherwise. If @var{buffer} is not supplied, the current buffer -is tested. +This function returns non-@code{nil} if the buffer @var{buffer} has +been modified since it was last read in from a file or saved, or +@code{nil} otherwise. If @var{buffer} has been autosaved after +@var{buffer} was last modified, the symbol @code{autosaved} is +returned. If @var{buffer} is not supplied, the current buffer is +tested. @end defun @defun set-buffer-modified-p flag diff --git a/lisp/subr.el b/lisp/subr.el index 01549cc6f7..54c9f35264 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -4594,21 +4594,17 @@ with-silent-modifications someone else, running buffer modification hooks, and other things of that nature." (declare (debug t) (indent 0)) - (let ((modified (make-symbol "modified")) - (tick (make-symbol "tick"))) + (let ((modified (make-symbol "modified"))) `(let* ((,modified (buffer-modified-p)) - (,tick (buffer-modified-tick)) (buffer-undo-list t) (inhibit-read-only t) (inhibit-modification-hooks t)) (unwind-protect (progn ,@body) - ;; We restore the buffer tick count, too, because otherwise - ;; we'll trigger a new auto-save. - (internal--set-buffer-modified-tick ,tick) - (unless ,modified - (restore-buffer-modified-p nil)))))) + (when (or (not ,modified) + (eq ,modified 'autosaved)) + (restore-buffer-modified-p ,modified)))))) (defmacro with-output-to-string (&rest body) "Execute BODY, return the text it sent to `standard-output', as a string." diff --git a/src/buffer.c b/src/buffer.c index 6334e197f0..d830943994 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -1376,12 +1376,23 @@ DEFUN ("buffer-local-variables", Fbuffer_local_variables, \f DEFUN ("buffer-modified-p", Fbuffer_modified_p, Sbuffer_modified_p, 0, 1, 0, - doc: /* Return t if BUFFER was modified since its file was last read or saved. -No argument or nil as argument means use current buffer as BUFFER. */) + doc: /* Return non-nil if BUFFER was modified since its file was last read or saved. +No argument or nil as argument means use current buffer as BUFFER. + +If BUFFER has been autosaved after BUFFER was last modified, the +symbol `autosaved' is returned. */) (Lisp_Object buffer) { struct buffer *buf = decode_buffer (buffer); - return BUF_SAVE_MODIFF (buf) < BUF_MODIFF (buf) ? Qt : Qnil; + if (BUF_SAVE_MODIFF (buf) < BUF_MODIFF (buf)) + { + if (BUF_AUTOSAVE_MODIFF (buf) == BUF_MODIFF (buf)) + return Qautosaved; + else + return Qt; + } + else + return Qnil; } DEFUN ("force-mode-line-update", Fforce_mode_line_update, @@ -1475,16 +1486,19 @@ DEFUN ("restore-buffer-modified-p", Frestore_buffer_modified_p, recent-auto-save-p from t to nil. Vice versa, if FLAG is non-nil and SAVE_MODIFF>=auto_save_modified we risk changing recent-auto-save-p from nil to t. */ - SAVE_MODIFF = (NILP (flag) - /* FIXME: This unavoidably sets recent-auto-save-p to nil. */ - ? MODIFF - /* Let's try to preserve recent-auto-save-p. */ - : SAVE_MODIFF < MODIFF ? SAVE_MODIFF - /* If SAVE_MODIFF == auto_save_modified == MODIFF, - we can either decrease SAVE_MODIFF and auto_save_modified - or increase MODIFF. */ - : modiff_incr (&MODIFF)); - + if (NILP (flag)) + /* This unavoidably sets recent-auto-save-p to nil. */ + SAVE_MODIFF = MODIFF; + else + { + if (EQ (flag, Qautosaved)) + BUF_AUTOSAVE_MODIFF (b) = MODIFF; + /* If SAVE_MODIFF == auto_save_modified == MODIFF, we can either + decrease SAVE_MODIFF and auto_save_modified or increase + MODIFF. */ + else if (SAVE_MODIFF >= MODIFF) + SAVE_MODIFF = modiff_incr (&MODIFF); + } return flag; } @@ -6465,5 +6479,7 @@ Functions (implicitly) running this hook are `get-buffer-create', defsubr (&Soverlay_put); defsubr (&Srestore_buffer_modified_p); + DEFSYM (Qautosaved, "autosaved"); + Fput (intern_c_string ("erase-buffer"), Qdisabled, Qt); } -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-09 17:40 ` Lars Ingebrigtsen @ 2022-05-09 18:29 ` Stefan Monnier 2022-05-10 1:51 ` Lars Ingebrigtsen 0 siblings, 1 reply; 25+ messages in thread From: Stefan Monnier @ 2022-05-09 18:29 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, emacs-devel > - ;; We restore the buffer tick count, too, because otherwise > - ;; we'll trigger a new auto-save. > - (internal--set-buffer-modified-tick ,tick) > - (unless ,modified > - (restore-buffer-modified-p nil)))))) > + (when (or (not ,modified) > + (eq ,modified 'autosaved)) > + (restore-buffer-modified-p ,modified)))))) [ I'd use `(unless (eq ,modified t)` ] But... hmm... what if `body` caused the modified-p status to change from `autosaved` to `nil`. With the old code, we'd leave `nil` untouched, but with the new code we'd re-set it to `autosaved`. Admittedly, it's quite unlikely for `body` to save the buffer (and hence cause the modified-p status to change from `autosaved` to `nil`), so I'm not sure how important this is. > - return BUF_SAVE_MODIFF (buf) < BUF_MODIFF (buf) ? Qt : Qnil; > + if (BUF_SAVE_MODIFF (buf) < BUF_MODIFF (buf)) > + { > + if (BUF_AUTOSAVE_MODIFF (buf) == BUF_MODIFF (buf)) > + return Qautosaved; > + else > + return Qt; > + } > + else > + return Qnil; I'd use ?: for both tests :-) > + else > + { > + if (EQ (flag, Qautosaved)) > + BUF_AUTOSAVE_MODIFF (b) = MODIFF; > + /* If SAVE_MODIFF == auto_save_modified == MODIFF, we can either > + decrease SAVE_MODIFF and auto_save_modified or increase > + MODIFF. */ > + else if (SAVE_MODIFF >= MODIFF) > + SAVE_MODIFF = modiff_incr (&MODIFF); > + } I think this will not do the right thing if you call `(restore-buffer-modified-p 'autosaved)` when modified-p is `nil`. Stefan ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-09 18:29 ` Stefan Monnier @ 2022-05-10 1:51 ` Lars Ingebrigtsen 2022-05-10 2:11 ` Stefan Monnier 0 siblings, 1 reply; 25+ messages in thread From: Lars Ingebrigtsen @ 2022-05-10 1:51 UTC (permalink / raw) To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> + (when (or (not ,modified) >> + (eq ,modified 'autosaved)) >> + (restore-buffer-modified-p ,modified)))))) > > [ I'd use `(unless (eq ,modified t)` ] The return value is now defined as nil/`autosaved'/non-nil, so assuming t is no longer the thing. > But... hmm... what if `body` caused the modified-p status to change from > `autosaved` to `nil`. With the old code, we'd leave `nil` untouched, > but with the new code we'd re-set it to `autosaved`. > > Admittedly, it's quite unlikely for `body` to save the buffer (and hence > cause the modified-p status to change from `autosaved` to `nil`), so I'm > not sure how important this is. I think that's probably outside the scope for this macro. >> + if (EQ (flag, Qautosaved)) >> + BUF_AUTOSAVE_MODIFF (b) = MODIFF; >> + /* If SAVE_MODIFF == auto_save_modified == MODIFF, we can either >> + decrease SAVE_MODIFF and auto_save_modified or increase >> + MODIFF. */ >> + else if (SAVE_MODIFF >= MODIFF) >> + SAVE_MODIFF = modiff_incr (&MODIFF); >> + } > > I think this will not do the right thing if you call > `(restore-buffer-modified-p 'autosaved)` when modified-p is `nil`. Yeah, the semantics are slightly obscure now, but you're not supposed to call the function with `autosaved' when it hasn't been modified. Perhaps it should just signal an error in that case? It might also make sense to just add a new function that only does auto-save modiff stuff, because things are somewhat un-orthogonal now. restore-buffer-modified-p can set modification status to nil/t, but can't set autosave status to t, only to nil. But anyway, I've now pushed the changes. (And will be removing internal--set-buffer-modified-tick a bit later, but I'm gonna wait a bit, because removing it will require everybody to say "make boostrap".) -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-10 1:51 ` Lars Ingebrigtsen @ 2022-05-10 2:11 ` Stefan Monnier 2022-05-10 11:45 ` Lars Ingebrigtsen 0 siblings, 1 reply; 25+ messages in thread From: Stefan Monnier @ 2022-05-10 2:11 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, emacs-devel >> [ I'd use `(unless (eq ,modified t)` ] > The return value is now defined as nil/`autosaved'/non-nil, so assuming > t is no longer the thing. :-) >> But... hmm... what if `body` caused the modified-p status to change from >> `autosaved` to `nil`. With the old code, we'd leave `nil` untouched, >> but with the new code we'd re-set it to `autosaved`. >> >> Admittedly, it's quite unlikely for `body` to save the buffer (and hence >> cause the modified-p status to change from `autosaved` to `nil`), so I'm >> not sure how important this is. > > I think that's probably outside the scope for this macro. Fair enough. In that case, maybe it can even call `restore-buffer-modified-p` unconditionally. >> I think this will not do the right thing if you call >> `(restore-buffer-modified-p 'autosaved)` when modified-p is `nil`. > > Yeah, the semantics are slightly obscure now, but you're not supposed to > call the function with `autosaved' when it hasn't been modified. > Perhaps it should just signal an error in that case? Why not make it DTRT and make sure (buffer-modified-p (restore-buffer-modified-p FLAG)) always returns FLAG if FLAG is one of nil/t/autosaved (and add a few tests while we're at it)? Stefan ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-10 2:11 ` Stefan Monnier @ 2022-05-10 11:45 ` Lars Ingebrigtsen 2022-05-10 11:55 ` Stefan Monnier 0 siblings, 1 reply; 25+ messages in thread From: Lars Ingebrigtsen @ 2022-05-10 11:45 UTC (permalink / raw) To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: > Fair enough. In that case, maybe it can even call > `restore-buffer-modified-p` unconditionally. Probably, but I don't think it makes much difference. >> Yeah, the semantics are slightly obscure now, but you're not supposed to >> call the function with `autosaved' when it hasn't been modified. >> Perhaps it should just signal an error in that case? > > Why not make it DTRT and make sure > > (buffer-modified-p (restore-buffer-modified-p FLAG)) > > always returns FLAG if FLAG is one of nil/t/autosaved (and add a few > tests while we're at it)? I'm actually not quite sure what the semantics should be if the buffer is buffer-modified-p => nil and we're called with (restore-buffer-modified-p 'autosaved)... -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-10 11:45 ` Lars Ingebrigtsen @ 2022-05-10 11:55 ` Stefan Monnier 2022-05-11 11:22 ` Lars Ingebrigtsen 0 siblings, 1 reply; 25+ messages in thread From: Stefan Monnier @ 2022-05-10 11:55 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, emacs-devel > I'm actually not quite sure what the semantics should be if the buffer > is buffer-modified-p => nil and we're called with > (restore-buffer-modified-p 'autosaved)... (restore-buffer-modified-p 'autosaved) can be done by doing the same as what happens if you do (restore-buffer-modified-p 'autosaved) and then you wait for auto-save to kick in, no? Stefan ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-10 11:55 ` Stefan Monnier @ 2022-05-11 11:22 ` Lars Ingebrigtsen 2022-05-11 13:19 ` Stefan Monnier 0 siblings, 1 reply; 25+ messages in thread From: Lars Ingebrigtsen @ 2022-05-11 11:22 UTC (permalink / raw) To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >> I'm actually not quite sure what the semantics should be if the buffer >> is buffer-modified-p => nil and we're called with >> (restore-buffer-modified-p 'autosaved)... > > (restore-buffer-modified-p 'autosaved) can be done by doing > the same as what happens if you do (restore-buffer-modified-p > 'autosaved) and then you wait for auto-save to kick in, no? Uhm... I don't follow. If somebody does (restore-buffer-modified-p 'autosaved), then that buffer won't be autosaved... (Well, until the user modifies the buffer.) -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-11 11:22 ` Lars Ingebrigtsen @ 2022-05-11 13:19 ` Stefan Monnier 2022-05-12 0:18 ` Lars Ingebrigtsen 0 siblings, 1 reply; 25+ messages in thread From: Stefan Monnier @ 2022-05-11 13:19 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, emacs-devel Lars Ingebrigtsen [2022-05-11 13:22:46] wrote: > Stefan Monnier <monnier@iro.umontreal.ca> writes: >>> I'm actually not quite sure what the semantics should be if the buffer >>> is buffer-modified-p => nil and we're called with >>> (restore-buffer-modified-p 'autosaved)... >> >> (restore-buffer-modified-p 'autosaved) can be done by doing >> the same as what happens if you do (restore-buffer-modified-p >> 'autosaved) and then you wait for auto-save to kick in, no? > > Uhm... I don't follow. If somebody does > (restore-buffer-modified-p 'autosaved), then that buffer won't be > autosaved... (Well, until the user modifies the buffer.) I'm saying that there should be no difficulty making (restore-buffer-modified-p 'autosaved) do the right thing since (restore-buffer-modified-p t) followed by auto-saving gives us the resulting state (of <FOO>_MODIFF vars) we want: we just need to mimic the behavior of that sequence of calls (but without performing the auto-save itself, of course). Stefan ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-11 13:19 ` Stefan Monnier @ 2022-05-12 0:18 ` Lars Ingebrigtsen 0 siblings, 0 replies; 25+ messages in thread From: Lars Ingebrigtsen @ 2022-05-12 0:18 UTC (permalink / raw) To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: > I'm saying that there should be no difficulty making > (restore-buffer-modified-p 'autosaved) do the right thing since > (restore-buffer-modified-p t) followed by auto-saving gives us the > resulting state (of <FOO>_MODIFF vars) we want: we just need to mimic > the behavior of that sequence of calls (but without performing the > auto-save itself, of course). I was confusing myself here -- I was thinking there was some reason we wouldn't want to (re)mark the buffer as modified with a FLAG of `autosaved', but there isn't one, I think? So I've adjusted the logic now. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving 2022-05-09 16:05 ` Lars Ingebrigtsen 2022-05-09 16:30 ` Eli Zaretskii @ 2022-05-09 17:00 ` Lars Ingebrigtsen 1 sibling, 0 replies; 25+ messages in thread From: Lars Ingebrigtsen @ 2022-05-09 17:00 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel Lars Ingebrigtsen <larsi@gnus.org> writes: > I don't think adding an extra call to `recent-auto-save-p' to the macro > would be that bad, (D'oh. I misremembered what that function did; ignore that...) -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2022-05-12 0:18 UTC | newest] Thread overview: 25+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <165191796540.22789.3432288633082546349@vcs2.savannah.gnu.org> [not found] ` <20220507100605.B7CA7C051FF@vcs2.savannah.gnu.org> 2022-05-07 16:06 ` master e8488bcc9c: Avoid having font locking triggering unnecessary auto-saving Stefan Monnier 2022-05-07 16:10 ` Eli Zaretskii 2022-05-07 16:27 ` Stefan Monnier 2022-05-07 16:40 ` Eli Zaretskii 2022-05-07 17:21 ` Stefan Monnier 2022-05-07 17:26 ` Eli Zaretskii 2022-05-07 16:49 ` Lars Ingebrigtsen 2022-05-09 12:37 ` Lars Ingebrigtsen 2022-05-09 13:22 ` Stefan Monnier 2022-05-09 13:35 ` Lars Ingebrigtsen 2022-05-09 13:47 ` Lars Ingebrigtsen 2022-05-09 15:43 ` Stefan Monnier 2022-05-09 16:05 ` Lars Ingebrigtsen 2022-05-09 16:30 ` Eli Zaretskii 2022-05-09 16:45 ` Lars Ingebrigtsen 2022-05-09 17:40 ` Lars Ingebrigtsen 2022-05-09 18:29 ` Stefan Monnier 2022-05-10 1:51 ` Lars Ingebrigtsen 2022-05-10 2:11 ` Stefan Monnier 2022-05-10 11:45 ` Lars Ingebrigtsen 2022-05-10 11:55 ` Stefan Monnier 2022-05-11 11:22 ` Lars Ingebrigtsen 2022-05-11 13:19 ` Stefan Monnier 2022-05-12 0:18 ` Lars Ingebrigtsen 2022-05-09 17:00 ` Lars Ingebrigtsen
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.