bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#69232: 30.0.50; [PATCH] EWW history navigation gets caught in a loop


From: Jim Porter
Subject: bug#69232: 30.0.50; [PATCH] EWW history navigation gets caught in a loop
Date: Fri, 1 Mar 2024 12:13:08 -0800

On 2/29/2024 11:26 PM, Eli Zaretskii wrote:
Thanks.  I have a usability comment below.

Thanks for taking a look.

"historical" here should be in quotes (and perhaps explained in
parentheses), otherwise people might misinterpret or fail to
understand what it alludes to.

Done. I've also added a short description about this to the EWW manual so that people in the future know they can tweak this behavior.

+(defcustom eww-before-browse-function #'eww-clear-future-history
+  "A function to call before browsing to a new page.
+By default, this removes any entries in the history that come after the
+current page (see `eww-clear-future-history')."

The doc string should describe the known possible values of the
option.

Done.

Now for the usability issue: It's okay to have this is a variable (to
allow future extensions, which might be unrelated to history), but
having a function for user option, and one that is more general than
being about history traversal, is not the best ides: it makes
customization harder for users who are not Lisp programmers.

After sleeping on this, I think I agree. I had debated whether to add something like 'eww-before-browse-hook', but then I reasoned that I really want exactly *one* of these history-modification functions to run before browsing, so I made it '...-function'. But that's not right either, since it would mess things up if someone wanted to use this as a hook (they'd need to write their own Lisp function to do everything).

There might be some use in the future for 'eww-before-browse-hook', but I can't think of any off the top of my head, so I've just changed this to 'eww-before-browse-history-function'. If we want a more general hook later, we can always add it when we know more.

Therefore, I suggest a history-specific defcustom, whose possible
values could be 'clear', 'clone', and 'add', and whose effect will be
to call the corresponding function via eww-before-browse-function.
The defcustom could also have a user-defined function value to allow
additional functions, of course.

I've done something close to this, though I retained the previous form where the possible values are things like 'eww-delete-future-history'. That keeps the code to *use* this option simpler (no special-case symbol names), and the only difference with your suggestion is that users who manually edit their init.el will have a longer symbol name to type.

For Customize users, I added 'function-item' choices so they should just be able to click on the option they want without having to worry about the exact symbol name.

If you strongly prefer to have this accept non-function symbol names like 'clear', let me know and I can change it. I don't care too much, but this way seemed simpler and easier to maintain.

Attachment: 0001-When-navigating-through-history-in-EWW-don-t-keep-ad.patch
Description: Text document


reply via email to

[Prev in Thread] Current Thread [Next in Thread]