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.