From: Juri Linkov <juri@linkov.net>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 69511@debbugs.gnu.org
Subject: bug#69511: Restore any state after revert-buffer
Date: Sun, 03 Mar 2024 19:28:34 +0200 [thread overview]
Message-ID: <86il23kywt.fsf@mail.linkov.net> (raw)
In-Reply-To: <86jzmjofdv.fsf@gnu.org> (Eli Zaretskii's message of "Sun, 03 Mar 2024 11:04:28 +0200")
>> > 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?
next prev parent reply other threads:[~2024-03-03 17:28 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-02 17:55 bug#69511: Restore any state after revert-buffer Juri Linkov
2024-03-02 18:11 ` Eli Zaretskii
2024-03-03 7:59 ` Juri Linkov
2024-03-03 9:04 ` Eli Zaretskii
2024-03-03 17:28 ` Juri Linkov [this message]
2024-03-03 17:43 ` Eli Zaretskii
2024-03-03 17:55 ` Juri Linkov
2024-03-03 18:46 ` Eli Zaretskii
2024-06-03 6:35 ` Juri Linkov
2024-06-03 16:55 ` Juri Linkov
2024-03-03 2:46 ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-03-03 7:55 ` Juri Linkov
2024-03-03 9:03 ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-06-04 6:37 ` Juri Linkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86il23kywt.fsf@mail.linkov.net \
--to=juri@linkov.net \
--cc=69511@debbugs.gnu.org \
--cc=eliz@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.