[PATCH] EWW - use revert--buffer-function to reload, and allow reload in eww-list-buffer

[PATCH] EWW - use revert--buffer-function to reload, and allow reload in eww-list-buffer

[Sebastián Monía]

---
Small quality of life change in EWW. I was interested in reverting the
buffer list, then I found the "FIXME" note about reverting in
eww-mode, and figured, why not :)

I didn't want to alter eww-reload (since it is part of the public API)
hence the new function for reverting that just calls it.

Thank you,
Seb

 lisp/net/eww.el | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index b2e1c5a72e5..c0ab8761997 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -1193,7 +1193,6 @@ This consults the entries in `eww-readable-urls' (which see)."
           (throw 'found result))))))
 
 (defvar-keymap eww-mode-map
-  "g" #'eww-reload             ;FIXME: revert-buffer-function instead!
   "G" #'eww
   "M-RET" #'eww-open-in-new-buffer
   "TAB" #'shr-next-link
@@ -1331,6 +1330,7 @@ within text input fields."
   (add-hook 'context-menu-functions 'eww-context-menu 5 t)
   (setq-local eww-history nil)
   (setq-local eww-history-position 0)
+  (setq-local revert-buffer-function #'eww--revert-function)
   (when (boundp 'tool-bar-map)
     (setq-local tool-bar-map eww-tool-bar-map))
   ;; desktop support
@@ -1529,6 +1529,13 @@ just re-display the HTML already fetched."
             (eww-retrieve url #'eww-render
 		          (list url (point) (current-buffer) encode))))))))
 
+(defun eww--revert-function (local _noconfirm)
+  "Revert function for EWW buffers.
+LOCAL works like in `eww-reload': when non-nil, reload the page from the
+network instead of the HTML already retrieved. It is the prefix arg."
+  (eww-reload local)
+  (message "Page reloaded."))
+
 ;; Form support.
 
 (defvar eww-form nil)
@@ -2601,8 +2608,10 @@ see)."
 
 ;;; eww buffers list
 
-(defun eww-list-buffers ()
-  "Enlist eww buffers."
+(defun eww-list-buffers (&optional _ignore-auto _noconfirm)
+  "Pop a buffer with a list of eww buffers.
+Optional arguments make this function compatible with the
+`revert-buffer-function' interface."
   (interactive)
   (let (buffers-info
         (current (current-buffer)))
@@ -2621,6 +2630,7 @@ see)."
           (domain-length 0)
           (title-length 0)
           url title format start)
+      (setq-local revert-buffer-function #'eww-list-buffers)
       (erase-buffer)
       (dolist (buffer-info buffers-info)
         (setq title-length (max title-length
-- 
2.45.2.windows.1
-- 
Sebastián Monía
https://site.sebasmonia.com/

Re: [PATCH] EWW - use revert--buffer-function to reload, and allow reload in eww-list-buffer

[Eli Zaretskii]

> From: Sebastián Monía <sebastian@sebasmonia.com>
> Date: Fri, 16 Aug 2024 13:28:54 -0400
> 
> Small quality of life change in EWW. I was interested in reverting the
> buffer list, then I found the "FIXME" note about reverting in
> eww-mode, and figured, why not :)
> 
> I didn't want to alter eww-reload (since it is part of the public API)
> hence the new function for reverting that just calls it.

Is it really TRT to call eww-reload?  The FIXME says we need a real
revert-buffer-function, so I presume just calling eww-reload is
somehow not the best solution?

In any case, what is the improvement provided by this change?

Thanks.

Re: [PATCH] EWW - use revert--buffer-function to reload, and allow reload in eww-list-buffer

[Sebastián Monía]

Eli Zaretskii <eliz@gnu.org> writes:

For reloading EWW buffers, I guess not much, except for using the
"standard" machinery to revert buffers. My first version changed
eww-reload instead, but then I realized someone might be already be
using that command.

The revert/reload for EWW buffer list is convenient. Without it, you get
a message that the buffer is not backed by a file.

I am fine keeping only buffer list change :)


>> From: Sebastián Monía <sebastian@sebasmonia.com>
>> Date: Fri, 16 Aug 2024 13:28:54 -0400
>> 
>> Small quality of life change in EWW. I was interested in reverting the
>> buffer list, then I found the "FIXME" note about reverting in
>> eww-mode, and figured, why not :)
>> 
>> I didn't want to alter eww-reload (since it is part of the public API)
>> hence the new function for reverting that just calls it.
>
> Is it really TRT to call eww-reload?  The FIXME says we need a real
> revert-buffer-function, so I presume just calling eww-reload is
> somehow not the best solution?
>
> In any case, what is the improvement provided by this change?
>
> Thanks.

-- 
Sebastián Monía
https://site.sebasmonia.com/

Re: [PATCH] EWW - use revert--buffer-function to reload, and allow reload in eww-list-buffer

[Sebastián Monía]

Sebastián Monía <sebastian@sebasmonia.com> writes:

---
This smaller patch does only the eww buffer list portion, and we leave
EWW buffer reloads as they are now.

 lisp/net/eww.el | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index b2e1c5a72e5..660731faa30 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -2601,8 +2601,10 @@ see)."
 
 ;;; eww buffers list
 
-(defun eww-list-buffers ()
-  "Enlist eww buffers."
+(defun eww-list-buffers (&optional _ignore-auto _noconfirm)
+  "Pop a buffer with a list of eww buffers.
+Optional arguments make this function compatible with the
+`revert-buffer-function' interface."
   (interactive)
   (let (buffers-info
         (current (current-buffer)))
@@ -2719,6 +2721,7 @@ see)."
 \\{eww-buffers-mode-map}"
   :interactive nil
   (buffer-disable-undo)
+  (setq-local revert-buffer-function #'eww-list-buffers)
   (setq truncate-lines t))
 
 ;;; Desktop support
-- 
2.45.2.windows.1





-- 
Sebastián Monía
https://site.sebasmonia.com/

Re: [PATCH] EWW - use revert--buffer-function to reload, and allow reload in eww-list-buffer

[Eli Zaretskii]

> From: Sebastián Monía <sebastian@sebasmonia.com>
> Cc: emacs-devel@gnu.org
> Date: Sat, 17 Aug 2024 08:35:36 -0400
> 
> Sebastián Monía <sebastian@sebasmonia.com> writes:
> 
> ---
> This smaller patch does only the eww buffer list portion, and we leave
> EWW buffer reloads as they are now.
> 
>  lisp/net/eww.el | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/lisp/net/eww.el b/lisp/net/eww.el
> index b2e1c5a72e5..660731faa30 100644
> --- a/lisp/net/eww.el
> +++ b/lisp/net/eww.el
> @@ -2601,8 +2601,10 @@ see)."
>  
>  ;;; eww buffers list
>  
> -(defun eww-list-buffers ()
> -  "Enlist eww buffers."
> +(defun eww-list-buffers (&optional _ignore-auto _noconfirm)
> +  "Pop a buffer with a list of eww buffers.
> +Optional arguments make this function compatible with the
> +`revert-buffer-function' interface."
>    (interactive)
>    (let (buffers-info
>          (current (current-buffer)))
> @@ -2719,6 +2721,7 @@ see)."
>  \\{eww-buffers-mode-map}"
>    :interactive nil
>    (buffer-disable-undo)
> +  (setq-local revert-buffer-function #'eww-list-buffers)
>    (setq truncate-lines t))
>  
>  ;;; Desktop support
> -- 
> 2.45.2.windows.1

Jim, would you please review this and provide comments, if any?

Thanks.

Re: [PATCH] EWW - use revert--buffer-function to reload, and allow reload in eww-list-buffer

[Jim Porter]

On 8/24/2024 1:42 AM, Eli Zaretskii wrote:
> Jim, would you please review this and provide comments, if any?

Hmm, it seems strange to me that this patch makes 'eww-list-buffers' be 
the 'revert-buffer-function'. That function contains some additional 
logic that I don't think belongs for reverting the buffer. For example, 
if I opened the EWW buffer list and then renamed the buffer, I'd expect 
'revert-buffer' to update the contents of that (now-renamed) buffer. 
 From reading the code, I think what would happen is that it pops to a 
*new* buffer with the original "*eww buffers*" name.

So I think we'd want some new function like 'eww--do-list-buffers' that 
takes the code from 'eww-list-buffers' that actually writes out the text 
(i.e. the 'let' form starting with '(inhibit-read-only t)'). Then have 
'eww-list-buffers' call that, and set 'revert-buffer-function' to 
'eww--do-list-buffers'. Or something like that anyway...

Re: [PATCH] EWW - use revert--buffer-function to reload, and allow reload in eww-list-buffer

[Sebastián Monía]

Hi Jim,

Thanks for your review.

On Sat, Aug 24, 2024, at 1:27 PM, Jim Porter wrote:
> So I think we'd want some new function like 'eww--do-list-buffers' that 
> takes the code from 'eww-list-buffers' that actually writes out the text 
> (i.e. the 'let' form starting with '(inhibit-read-only t)'). Then have 
> 'eww-list-buffers' call that, and set 'revert-buffer-function' to 
> 'eww--do-list-buffers'. Or something like that anyway...

You are entirely correct, will provide an updated patch whenever I get a chance.
Thank you,
Seb

Re: [PATCH] EWW - use revert--buffer-function to reload, and allow reload in eww-list-buffer

[Eli Zaretskii]

> Date: Fri, 30 Aug 2024 16:01:00 -0400
> From: Sebastián Monía <sebastian@sebasmonia.com>
> Cc: emacs-devel@gnu.org
> 
> Hi Jim,
> 
> Thanks for your review.
> 
> On Sat, Aug 24, 2024, at 1:27 PM, Jim Porter wrote:
> > So I think we'd want some new function like 'eww--do-list-buffers' that 
> > takes the code from 'eww-list-buffers' that actually writes out the text 
> > (i.e. the 'let' form starting with '(inhibit-read-only t)'). Then have 
> > 'eww-list-buffers' call that, and set 'revert-buffer-function' to 
> > 'eww--do-list-buffers'. Or something like that anyway...
> 
> You are entirely correct, will provide an updated patch whenever I get a chance.

Any progress there?