bug#69511: Restore any state after revert-buffer

bug#69511: Restore any state after revert-buffer

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 169 bytes --]

This patch adds a new variable 'revert-buffer-state-functions'
that will allow any state to be saved and restored,
not only the currently hard-coded 'read-only' state:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: revert-buffer-state-functions.patch --]
[-- Type: text/x-diff, Size: 1704 bytes --]

diff --git a/lisp/files.el b/lisp/files.el
index ed18bc5841e..e8ecb351759 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -6804,6 +6805,13 @@ revert-buffer-internal-hook
 ;; `preserve-modes' argument of `revert-buffer'.
 (defvar revert-buffer-preserve-modes)
 
+(defvar-local revert-buffer-state-functions nil
+  "Functions to save and restore any state during `revert-buffer'.
+This variable is a list of functions that are called before
+reverting the buffer.  These functions should return a lambda
+that will be called after reverting the buffer
+to restore a previous state saved in that lambda.")
+
 (defun revert-buffer (&optional ignore-auto noconfirm preserve-modes)
   "Replace current buffer text with the text of the visited file on disk.
 This undoes all changes since the file was visited or saved.
@@ -6854,13 +6862,16 @@ revert-buffer
   (let ((revert-buffer-in-progress-p t)
         (revert-buffer-preserve-modes preserve-modes)
         (state (and (boundp 'read-only-mode--state)
-                    (list read-only-mode--state))))
+                    (list read-only-mode--state)))
+        (state-functions
+         (delq nil (mapcar #'funcall revert-buffer-state-functions))))
     ;; Return whatever 'revert-buffer-function' returns.
     (prog1 (funcall (or revert-buffer-function #'revert-buffer--default)
                     ignore-auto noconfirm)
       (when state
         (setq buffer-read-only (car state))
-        (setq-local read-only-mode--state (car state))))))
+        (setq-local read-only-mode--state (car state)))
+      (mapc #'funcall state-functions))))
 
 (defun revert-buffer--default (ignore-auto noconfirm)
   "Default function for `revert-buffer'.

[-- Attachment #3: Type: text/plain, Size: 691 bytes --]


One of numerous examples is to save and restore
hidden outlines after doing 'revert-buffer':

(add-hook 'Buffer-menu-mode-hook
          (lambda ()
            (add-hook 'revert-buffer-state-functions
                      (lambda ()
                        (let ((regexp (outline-hidden-headings-regexp)))
                          (when regexp
                            (lambda ()
                              (outline-hide-by-heading-regexp regexp))))))))

Currently this handles outlines only in the buffer list.
But probably this settings should be added to outline-minor-mode.

And here is the immediate need to use it to rehighlight the outlines
after reverting the buffer list:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: outline-minor-mode-highlight-buffer.patch --]
[-- Type: text/x-diff, Size: 702 bytes --]

diff --git a/lisp/buff-menu.el b/lisp/buff-menu.el
index ec5337e3fda..ae3f3e0035e 100644
--- a/lisp/buff-menu.el
+++ b/lisp/buff-menu.el
@@ -274,7 +276,12 @@ Buffer-menu-mode
   :interactive nil
   (setq-local buffer-stale-function
               (lambda (&optional _noconfirm) 'fast))
-  (add-hook 'tabulated-list-revert-hook 'list-buffers--refresh nil t))
+  (add-hook 'tabulated-list-revert-hook 'list-buffers--refresh nil t)
+  (add-hook 'revert-buffer-state-functions
+            (lambda ()
+              (when (bound-and-true-p outline-minor-mode)
+                (lambda ()
+                  (outline-minor-mode-highlight-buffer))))))
 
 (defun buffer-menu--display-help ()
   (message "%s"

bug#69511: Restore any state after revert-buffer

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Sat, 02 Mar 2024 19:55:50 +0200
> 
> This patch adds a new variable 'revert-buffer-state-functions'
> that will allow any state to be saved and restored,
> not only the currently hard-coded 'read-only' state:

It isn't clear what you mean by "state" here.  It seems like you are
talking about functions to be called after reverting a buffer in order
to do whatever is appropriate after reverting.  And if so, why
"state"?  For example, one of your examples, with
outline-minor-mode-highlight-buffer, is not about "state", but about
re-highlighting the buffer after reverting it.

So I would suggest calling this "revert-buffer-post-revert-functions",
and updating the doc string to make it clear that the purpose is more
general.

> +(defvar-local revert-buffer-state-functions nil
> +  "Functions to save and restore any state during `revert-buffer'.
> +This variable is a list of functions that are called before
> +reverting the buffer.  These functions should return a lambda
> +that will be called after reverting the buffer
> +to restore a previous state saved in that lambda.")

The last sentence needs to be reworded, because it is not clear how
functions (in plural) can return a lambda (in singular).  Also, the
doc string should describe how the function is called (with no
arguments, I guess?), and that all those functions will be called one
by one in the order of the list.

Finally, this needs to be documented in NEWS and the ELisp reference
manual.

Thanks.

bug#69511: Restore any state after revert-buffer

[Juri Linkov]

>> This patch adds a new variable 'revert-buffer-state-functions'
>> that will allow any state to be saved and restored,
>> not only the currently hard-coded 'read-only' state:
>
> It isn't clear what you mean by "state" here.  It seems like you are
> talking about functions to be called after reverting a buffer in order
> to do whatever is appropriate after reverting.  And if so, why
> "state"?  For example, one of your examples, with
> outline-minor-mode-highlight-buffer, is not about "state", but about
> re-highlighting the buffer after reverting it.

"state" here is in the same sense as is already used in 'revert-buffer':

  (let ((state (and (boundp 'read-only-mode--state)
                    (list read-only-mode--state))))
    (prog1 (funcall (or revert-buffer-function #'revert-buffer--default)
                    ignore-auto noconfirm)
      (when state
        (setq buffer-read-only (car state))
        (setq-local read-only-mode--state (car state)))))

> So I would suggest calling this "revert-buffer-post-revert-functions",
> and updating the doc string to make it clear that the purpose is more
> general.

"post" can't be used because these functions are called
before 'revert-buffer-function'.

"pre" would be better but still has no hint that the returned lambdas
will be called after 'revert-buffer-function' to restore a state.

>> +(defvar-local revert-buffer-state-functions nil
>> +  "Functions to save and restore any state during `revert-buffer'.
>> +This variable is a list of functions that are called before
>> +reverting the buffer.  These functions should return a lambda
>> +that will be called after reverting the buffer
>> +to restore a previous state saved in that lambda.")
>
> The last sentence needs to be reworded, because it is not clear how
> functions (in plural) can return a lambda (in singular).  Also, the
> doc string should describe how the function is called (with no
> arguments, I guess?), and that all those functions will be called one
> by one in the order of the list.

Here is an improved docstring:

(defvar-local revert-buffer-state-functions nil
  "Functions to save and restore any state during `revert-buffer'.
This variable is a list of functions that are called before reverting
the buffer.  Each of these functions are called without arguments and
should return a lambda that can restore a previous state of the buffer.
Then after reverting the buffer each of these lambdas will be called
one by one in the order of the list to restore previous states of the
buffer.  An example of the buffer state is keeping the buffer read-only,
or keeping minor modes, etc.")

> Finally, this needs to be documented in NEWS and the ELisp reference
> manual.

If this docstring is ok, then it could be adapted to the ELisp reference
manual.

bug#69511: Restore any state after revert-buffer

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 69511@debbugs.gnu.org
> Date: Sun, 03 Mar 2024 09:59:24 +0200
> 
> >> This patch adds a new variable 'revert-buffer-state-functions'
> >> that will allow any state to be saved and restored,
> >> not only the currently hard-coded 'read-only' state:
> >
> > It isn't clear what you mean by "state" here.  It seems like you are
> > talking about functions to be called after reverting a buffer in order
> > to do whatever is appropriate after reverting.  And if so, why
> > "state"?  For example, one of your examples, with
> > outline-minor-mode-highlight-buffer, is not about "state", but about
> > re-highlighting the buffer after reverting it.
> 
> "state" here is in the same sense as is already used in 'revert-buffer':

"Two wrongs don't make a right", as the saying goes.  And the name of
a local variable is much less significant than the name of a public
variable.

> > So I would suggest calling this "revert-buffer-post-revert-functions",
> > and updating the doc string to make it clear that the purpose is more
> > general.
> 
> "post" can't be used because these functions are called
> before 'revert-buffer-function'.

According to your doc string, the lambdas these functions produce are
called _after_ reverting the buffer.  So I see no reason not to use
"post" here.

> "pre" would be better but still has no hint that the returned lambdas
> will be called after 'revert-buffer-function' to restore a state.

The doc string can make that evident, so that's not a problem IMO.
And again, "restore a state" doesn't describe what, for example,
outline-minor-mode-highlight-buffer does.  So this is not a good
source for a descriptive name of the variable.

> >> +(defvar-local revert-buffer-state-functions nil
> >> +  "Functions to save and restore any state during `revert-buffer'.
> >> +This variable is a list of functions that are called before
> >> +reverting the buffer.  These functions should return a lambda
> >> +that will be called after reverting the buffer
> >> +to restore a previous state saved in that lambda.")
> >
> > The last sentence needs to be reworded, because it is not clear how
> > functions (in plural) can return a lambda (in singular).  Also, the
> > doc string should describe how the function is called (with no
> > arguments, I guess?), and that all those functions will be called one
> > by one in the order of the list.
> 
> Here is an improved docstring:
> 
> (defvar-local revert-buffer-state-functions nil
>   "Functions to save and restore any state during `revert-buffer'.
> This variable is a list of functions that are called before reverting
  ^^^^^^^^^^^^^
"The value of this variable..."

> the buffer.  Each of these functions are called without arguments and
> should return a lambda that can restore a previous state of the buffer.
> Then after reverting the buffer each of these lambdas will be called
> one by one in the order of the list to restore previous states of the
> buffer.  An example of the buffer state is keeping the buffer read-only,
> or keeping minor modes, etc.")

Apart of the "state" thing, which I think is sub-optimal for the
reasons explained above, this is fine, thanks.

bug#69511: Restore any state after revert-buffer

[Juri Linkov]

>> > So I would suggest calling this "revert-buffer-post-revert-functions",
>> > and updating the doc string to make it clear that the purpose is more
>> > general.
>>
>> "post" can't be used because these functions are called
>> before 'revert-buffer-function'.
>
> According to your doc string, the lambdas these functions produce are
> called _after_ reverting the buffer.  So I see no reason not to use
> "post" here.

"revert-buffer-post-revert-functions" will be very confusing to users
because all hooks with such names are intended to contain functions
added by users and that are run afterwards.  Examples:

isearch-update-post-hook
ispell-update-post-hook
vc-post-command-functions

i.e. users add here functions that are called after command finishes.
Not in this case.

>> "pre" would be better but still has no hint that the returned lambdas
>> will be called after 'revert-buffer-function' to restore a state.
>
> The doc string can make that evident, so that's not a problem IMO.
> And again, "restore a state" doesn't describe what, for example,
> outline-minor-mode-highlight-buffer does.  So this is not a good
> source for a descriptive name of the variable.

outline-minor-mode-highlight-buffer is a very bad example.
I regret that I posted it because such exception should affect the design.
But even this abuse of state functions still restores the state.
It should have looked this way:

(add-hook 'revert-buffer-state-functions
          (lambda ()
            (when (bound-and-true-p outline-minor-mode)
              (lambda ()
                (outline-minor-mode))))))

Here clearly it saves and restores the minor mode.
Only that for some reason outline-minor-mode
is not disabled, so what it needs to do is
only to restore the state of highlighting.

>> >> +(defvar-local revert-buffer-state-functions nil
>> >> +  "Functions to save and restore any state during `revert-buffer'.
>> >> +This variable is a list of functions that are called before
>> >> +reverting the buffer.  These functions should return a lambda
>> >> +that will be called after reverting the buffer
>> >> +to restore a previous state saved in that lambda.")
>> >
>> > The last sentence needs to be reworded, because it is not clear how
>> > functions (in plural) can return a lambda (in singular).  Also, the
>> > doc string should describe how the function is called (with no
>> > arguments, I guess?), and that all those functions will be called one
>> > by one in the order of the list.
>> 
>> Here is an improved docstring:
>> 
>> (defvar-local revert-buffer-state-functions nil
>>   "Functions to save and restore any state during `revert-buffer'.
>> This variable is a list of functions that are called before reverting
>   ^^^^^^^^^^^^^
> "The value of this variable..."
>
>> the buffer.  Each of these functions are called without arguments and
>> should return a lambda that can restore a previous state of the buffer.
>> Then after reverting the buffer each of these lambdas will be called
>> one by one in the order of the list to restore previous states of the
>> buffer.  An example of the buffer state is keeping the buffer read-only,
>> or keeping minor modes, etc.")
>
> Apart of the "state" thing, which I think is sub-optimal for the
> reasons explained above, this is fine, thanks.

Like Michael suggested a set of functions should be created.
These functions should share the same prefix.

Then revert-buffer-state-read-only will looks like this:

diff --git a/lisp/files.el b/lisp/files.el
index 3460c327bb4..82ad748d192 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -6805,6 +6805,24 @@ revert-buffer-internal-hook
 ;; `preserve-modes' argument of `revert-buffer'.
 (defvar revert-buffer-preserve-modes)
 
+(defvar revert-buffer-state-functions '(revert-buffer-state-read-only)
+  "Functions to save and restore any state during `revert-buffer'.
+The value of this variable is a list of functions that are called before
+reverting the buffer.  Each of these functions are called without
+arguments and should return a lambda that can restore a previous state
+of the buffer.  Then after reverting the buffer each of these lambdas
+will be called one by one in the order of the list to restore previous
+states of the buffer.  An example of the buffer state is keeping the
+buffer read-only, or keeping minor modes, etc.")
+
+(defun revert-buffer-state-read-only ()
+  "Save and restore read-only state for `revert-buffer'."
+  (when-let ((state (and (boundp 'read-only-mode--state)
+                         (list read-only-mode--state))))
+    (lambda ()
+      (setq buffer-read-only (car state))
+      (setq-local read-only-mode--state (car state)))))
+
 (defun revert-buffer (&optional ignore-auto noconfirm preserve-modes)
   "Replace current buffer text with the text of the visited file on disk.
 This undoes all changes since the file was visited or saved.
@@ -6854,14 +6872,12 @@ revert-buffer
   (interactive (list (not current-prefix-arg)))
   (let ((revert-buffer-in-progress-p t)
         (revert-buffer-preserve-modes preserve-modes)
-        (state (and (boundp 'read-only-mode--state)
-                    (list read-only-mode--state))))
+        (state-functions
+         (delq nil (mapcar #'funcall revert-buffer-state-functions))))
     ;; Return whatever 'revert-buffer-function' returns.
     (prog1 (funcall (or revert-buffer-function #'revert-buffer--default)
                     ignore-auto noconfirm)
-      (when state
-        (setq buffer-read-only (car state))
-        (setq-local read-only-mode--state (car state))))))
+      (mapc #'funcall state-functions))))
 
 (defun revert-buffer--default (ignore-auto noconfirm)
   "Default function for `revert-buffer'.

So the whole set of functions will be:

revert-buffer-state-functions
revert-buffer-state-read-only
revert-buffer-state-outlines
...

Or like the variable 'preserve-modes' hints above,
the common prefix could be like this:

revert-buffer-preserve-functions
revert-buffer-preserve-modes
revert-buffer-preserve-read-only
revert-buffer-preserve-outlines
...

Which prefix would you prefer?

bug#69511: Restore any state after revert-buffer

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 69511@debbugs.gnu.org
> Date: Sun, 03 Mar 2024 19:28:34 +0200
> 
> > Apart of the "state" thing, which I think is sub-optimal for the
> > reasons explained above, this is fine, thanks.
> 
> Like Michael suggested a set of functions should be created.
> These functions should share the same prefix.
> 
> Then revert-buffer-state-read-only will looks like this:
> 
> diff --git a/lisp/files.el b/lisp/files.el
> index 3460c327bb4..82ad748d192 100644
> --- a/lisp/files.el
> +++ b/lisp/files.el
> @@ -6805,6 +6805,24 @@ revert-buffer-internal-hook
>  ;; `preserve-modes' argument of `revert-buffer'.
>  (defvar revert-buffer-preserve-modes)
>  
> +(defvar revert-buffer-state-functions '(revert-buffer-state-read-only)
> +  "Functions to save and restore any state during `revert-buffer'.
> +The value of this variable is a list of functions that are called before
> +reverting the buffer.  Each of these functions are called without
> +arguments and should return a lambda that can restore a previous state
> +of the buffer.  Then after reverting the buffer each of these lambdas
> +will be called one by one in the order of the list to restore previous
> +states of the buffer.  An example of the buffer state is keeping the
> +buffer read-only, or keeping minor modes, etc.")
> +
> +(defun revert-buffer-state-read-only ()
> +  "Save and restore read-only state for `revert-buffer'."
> +  (when-let ((state (and (boundp 'read-only-mode--state)
> +                         (list read-only-mode--state))))
> +    (lambda ()
> +      (setq buffer-read-only (car state))
> +      (setq-local read-only-mode--state (car state)))))
> +
>  (defun revert-buffer (&optional ignore-auto noconfirm preserve-modes)
>    "Replace current buffer text with the text of the visited file on disk.
>  This undoes all changes since the file was visited or saved.
> @@ -6854,14 +6872,12 @@ revert-buffer
>    (interactive (list (not current-prefix-arg)))
>    (let ((revert-buffer-in-progress-p t)
>          (revert-buffer-preserve-modes preserve-modes)
> -        (state (and (boundp 'read-only-mode--state)
> -                    (list read-only-mode--state))))
> +        (state-functions
> +         (delq nil (mapcar #'funcall revert-buffer-state-functions))))
>      ;; Return whatever 'revert-buffer-function' returns.
>      (prog1 (funcall (or revert-buffer-function #'revert-buffer--default)
>                      ignore-auto noconfirm)
> -      (when state
> -        (setq buffer-read-only (car state))
> -        (setq-local read-only-mode--state (car state))))))
> +      (mapc #'funcall state-functions))))
>  
>  (defun revert-buffer--default (ignore-auto noconfirm)
>    "Default function for `revert-buffer'.
> 
> So the whole set of functions will be:
> 
> revert-buffer-state-functions
> revert-buffer-state-read-only
> revert-buffer-state-outlines
> ...
> 
> Or like the variable 'preserve-modes' hints above,
> the common prefix could be like this:
> 
> revert-buffer-preserve-functions
> revert-buffer-preserve-modes
> revert-buffer-preserve-read-only
> revert-buffer-preserve-outlines
> ...
> 
> Which prefix would you prefer?

How about revert-buffer-restore- ?

bug#69511: Restore any state after revert-buffer

[Juri Linkov]

>> Like Michael suggested a set of functions should be created.
>> These functions should share the same prefix.
>> 
>> So the whole set of functions will be:
>> 
>> revert-buffer-state-functions
>> revert-buffer-state-read-only
>> revert-buffer-state-outlines
>> ...
>> 
>> Or like the variable 'preserve-modes' hints above,
>> the common prefix could be like this:
>> 
>> revert-buffer-preserve-functions
>> revert-buffer-preserve-modes
>> revert-buffer-preserve-read-only
>> revert-buffer-preserve-outlines
>> ...
>> 
>> Which prefix would you prefer?
>
> How about revert-buffer-restore- ?

"restore" would be only part of the truth,
because these functions first save the state,
only their second task is to restore the saved state
afterwards.

bug#69511: Restore any state after revert-buffer

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 69511@debbugs.gnu.org
> Date: Sun, 03 Mar 2024 19:55:15 +0200
> 
> >> Like Michael suggested a set of functions should be created.
> >> These functions should share the same prefix.
> >> 
> >> So the whole set of functions will be:
> >> 
> >> revert-buffer-state-functions
> >> revert-buffer-state-read-only
> >> revert-buffer-state-outlines
> >> ...
> >> 
> >> Or like the variable 'preserve-modes' hints above,
> >> the common prefix could be like this:
> >> 
> >> revert-buffer-preserve-functions
> >> revert-buffer-preserve-modes
> >> revert-buffer-preserve-read-only
> >> revert-buffer-preserve-outlines
> >> ...
> >> 
> >> Which prefix would you prefer?
> >
> > How about revert-buffer-restore- ?
> 
> "restore" would be only part of the truth,
> because these functions first save the state,
> only their second task is to restore the saved state
> afterwards.

IMO, "restore" is better than "state", because you don't really
restore any state: a buffer has no state, per se.  "Restore" is also
better than "preserve".

What is attractive in "restore" is that it is general and generic
enough to include all the meanings you have shown in your examples.
So I think "restore" is the best candidate till now.  If someone has
better suggestions, please speak up.

bug#69511: Restore any state after revert-buffer

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 1030 bytes --]

>> >> revert-buffer-preserve-functions
>> >> revert-buffer-preserve-modes
>> >> revert-buffer-preserve-read-only
>> >> revert-buffer-preserve-outlines
>> >
>> > How about revert-buffer-restore- ?
>>
>> "restore" would be only part of the truth,
>> because these functions first save the state,
>> only their second task is to restore the saved state
>> afterwards.
>
> IMO, "restore" is better than "state", because you don't really
> restore any state: a buffer has no state, per se.  "Restore" is also
> better than "preserve".
>
> What is attractive in "restore" is that it is general and generic
> enough to include all the meanings you have shown in your examples.
> So I think "restore" is the best candidate till now.  If someone has
> better suggestions, please speak up.

Since no one has better suggestions, here is the patch that uses
"restore".  This will create two different name prefixes:

  revert-buffer-preserve-modes
  revert-buffer-restore-read-only

but this is fine.  So here is a complete patch with docs:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: revert-buffer-restore-functions.patch --]
[-- Type: text/x-diff, Size: 3758 bytes --]

diff --git a/doc/lispref/backups.texi b/doc/lispref/backups.texi
index 8d0f3806646..a55b0a6aed2 100644
--- a/doc/lispref/backups.texi
+++ b/doc/lispref/backups.texi
@@ -777,6 +777,16 @@ Reverting
 may or may not run this hook.
 @end defvar
 
+@defvar revert-buffer-restore-functions
+The value of this variable specifies a list of functions that preserve
+the state of the buffer.  Before the revert operation each function from
+this list is called without arguments, and it should return a lambda
+that preserves some particular state (for example, the read-only state).
+After the revert operation each lambda will be called one by one in the
+order of the list, and it should restore the saved state in the reverted
+buffer.
+@end defvar
+
 Emacs can revert buffers automatically.  It does that by default for
 buffers visiting files.  The following describes how to add support
 for auto-reverting new types of buffers.
diff --git a/etc/NEWS b/etc/NEWS
index e5a63cc8a67..571ed359924 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2704,6 +2704,10 @@ treesitter grammar.
 ** New buffer-local variable 'tabulated-list-groups'.
 It controls display and separate sorting of groups of entries.
 
++++
+** New variable 'revert-buffer-restore-functions'.
+It helps to preserve various states after reverting the buffer.
+
 ---
 ** New text property 'context-menu-functions'.
 Like the variable with the same name, it adds menus from the list that
diff --git a/lisp/files.el b/lisp/files.el
index b25cca00bb3..210cd0fa7ad 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -6848,6 +6848,24 @@ revert-buffer-internal-hook
 ;; `preserve-modes' argument of `revert-buffer'.
 (defvar revert-buffer-preserve-modes)
 
+(defvar revert-buffer-restore-functions '(revert-buffer-restore-read-only)
+  "Functions to preserve any state during `revert-buffer'.
+The value of this variable is a list of functions that are called before
+reverting the buffer.  Each of these functions are called without
+arguments and should return a lambda that can restore a previous state
+of the buffer.  Then after reverting the buffer each of these lambdas
+will be called one by one in the order of the list to restore previous
+states of the buffer.  An example of the buffer state is keeping the
+buffer read-only, or keeping minor modes, etc.")
+
+(defun revert-buffer-restore-read-only ()
+  "Preserve read-only state for `revert-buffer'."
+  (when-let ((state (and (boundp 'read-only-mode--state)
+                         (list read-only-mode--state))))
+    (lambda ()
+      (setq buffer-read-only (car state))
+      (setq-local read-only-mode--state (car state)))))
+
 (defun revert-buffer (&optional ignore-auto noconfirm preserve-modes)
   "Replace current buffer text with the text of the visited file on disk.
 This undoes all changes since the file was visited or saved.
@@ -6897,14 +6915,13 @@ revert-buffer
   (interactive (list (not current-prefix-arg)))
   (let ((revert-buffer-in-progress-p t)
         (revert-buffer-preserve-modes preserve-modes)
-        (state (and (boundp 'read-only-mode--state)
-                    (list read-only-mode--state))))
+        restore-functions)
+    (run-hook-wrapped 'revert-buffer-restore-functions
+                      (lambda (f) (push (funcall f) restore-functions) nil))
     ;; Return whatever 'revert-buffer-function' returns.
     (prog1 (funcall (or revert-buffer-function #'revert-buffer--default)
                     ignore-auto noconfirm)
-      (when state
-        (setq buffer-read-only (car state))
-        (setq-local read-only-mode--state (car state))))))
+      (mapc #'funcall (delq nil restore-functions)))))
 
 (defun revert-buffer--default (ignore-auto noconfirm)
   "Default function for `revert-buffer'.

bug#69511: Restore any state after revert-buffer

[Juri Linkov]

close 69511 30.0.50
thanks

>> IMO, "restore" is better than "state", because you don't really
>> restore any state: a buffer has no state, per se.  "Restore" is also
>> better than "preserve".
>>
>> What is attractive in "restore" is that it is general and generic
>> enough to include all the meanings you have shown in your examples.
>> So I think "restore" is the best candidate till now.  If someone has
>> better suggestions, please speak up.
>
> Since no one has better suggestions, here is the patch that uses
> "restore".  This will create two different name prefixes:
>
>   revert-buffer-preserve-modes
>   revert-buffer-restore-read-only
>
> but this is fine.  So here is a complete patch with docs:

So now pushed, and closed.

bug#69511: Restore any state after revert-buffer

[Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Juri Linkov <juri@linkov.net> writes:

> +  (add-hook 'revert-buffer-state-functions
> +            (lambda ()
> +              (when (bound-and-true-p outline-minor-mode)
> +                (lambda ()
> +                  (outline-minor-mode-highlight-buffer))))))

Side note: most of the time it's better to avoid to add lambdas to hooks
for several reasons (non-trivial identity checks; inspection of the
variable by the user) when possible.

Michael.

bug#69511: Restore any state after revert-buffer

[Juri Linkov]

>> +  (add-hook 'revert-buffer-state-functions
>> +            (lambda ()
>> +              (when (bound-and-true-p outline-minor-mode)
>> +                (lambda ()
>> +                  (outline-minor-mode-highlight-buffer))))))
>
> Side note: most of the time it's better to avoid to add lambdas to hooks
> for several reasons (non-trivial identity checks; inspection of the
> variable by the user) when possible.

I remember Alan said this in bug#66750.

bug#69511: Restore any state after revert-buffer

[Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Juri Linkov <juri@linkov.net> writes:

> I remember Alan said this in bug#66750.

Yes, this is one aspect of the problem.

AFAIR we try to avoid adding lambdas to hooks in the Emacs sources
because when the user looks at the hook value, seeing a simple list of
symbols is easier to understand and nicer for the eye.

Michael.

bug#69511: Restore any state after revert-buffer

[Juri Linkov]

close 69511 30.0.50
thanks

> One of numerous examples is to save and restore
> hidden outlines after doing 'revert-buffer':
>
> (add-hook 'Buffer-menu-mode-hook
>           (lambda ()
>             (add-hook 'revert-buffer-state-functions
>                       (lambda ()
>                         (let ((regexp (outline-hidden-headings-regexp)))
>                           (when regexp
>                             (lambda ()
>                               (outline-hide-by-heading-regexp regexp))))))))
>
> Currently this handles outlines only in the buffer list.
> But probably this settings should be added to outline-minor-mode.

This is now adapted to outline-minor-mode.
(xref needs changes as well, so the patch is sent to bug#49731).

> And here is the immediate need to use it to rehighlight the outlines
> after reverting the buffer list:
>
> diff --git a/lisp/buff-menu.el b/lisp/buff-menu.el
> +  (add-hook 'tabulated-list-revert-hook 'list-buffers--refresh nil t)
> +  (add-hook 'revert-buffer-state-functions
> +            (lambda ()
> +              (when (bound-and-true-p outline-minor-mode)
> +                (lambda ()
> +                  (outline-minor-mode-highlight-buffer))))))

And this was moved to outline-minor-mode as well.