unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* 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: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
  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
       [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: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

* 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

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 public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).