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?