On 2019-02-08 08:52 +02, Eli Zaretskii <eliz@gnu.org> wrote:
> Wouldn't it be more convenient if you could invoke eww with a prefix
> argument for that?

Sure, appended a patch, can add docs and news modifications to it if you
like it better.

>> I have manually tested the general use of EWW with this patch applied.
>> But I haven’t found a test suite for EWW; if I missed it, I can run it,
>> or any other suggested testing.
>
> You could start a test suite, although testing eww should ideally work
> even if no network connection is available.

I can try to come up with tests for the feature I’m adding.  EWW works
with file:// links, so generating a few temporary files and working with
them should work alright.  I’m not familiar enough with Emacs TDD and
EWW to try add something more comprehensive though.

>> Subject: [PATCH] Support not reusing *eww* buffer when navigating from a
>>  non-eww one
>
> It is best to reword this header line to be positive instead of
> negative.
>
>> +(defcustom eww-reuse-buffer t
>> +  "Reuse the *eww* buffer when not in an `eww-mode' buffer."
>
> For a boolean option, the first line of the doc string should say
> either
>
>   Non-nil means reuse the *eww* buffer ...
>
> or
>
>   Whether to reuse the *eww* buffer ...
>
> (the former is preferable).

I can apply these to the old patch if you think that approach is better
than the one in the new patch (using a prefix arg), which doesn’t have
these problems.

>> +When the current buffer is not in `eww-mode', if
>> +`eww-reuse-buffer' is non-nil, the *eww* buffer will be reused if
>> +available, otherwise generated; if set to nil instead, a new
>> +buffer will be used in case *eww* is already in use."
>
> Once this feature exists and is used, wouldn't it be better to program
> it so it either reuses the current EWW buffer or creates a new one,
> regardless of whether the current buffer's name is "*eww*"?  IOW,
> should we really hardcode "*eww*" in this feature?

Sure.  The literal "*eww*" appears only in two places, and could be
replaced with a defcustom and/or an extra arg to ‘eww’.

I don’t think it depends on the name of the current buffer as it is,
e.g. M-x eww ... RET M-x rename-buffer somebuf RET C-u M-x eww ... RET
does work, resulting in buffers *eww*, somebuf, *eww*<2>, and
‘eww-list-buffers’ can show a user-friendly list of buffers.  Do you
mean this scenario should result in somebuf<2> instead of *eww*<2>?  I
think that’d be confusing when calling M-x eww from some non-eww buffer,
I’d rather have new names be based on the default buffer name.

>> -     (get-buffer-create "*eww*")))
>> +     (funcall
>> +      (if eww-reuse-buffer #'get-buffer-create #'generate-new-buffer)
>> +      "*eww*")))
>
> Any particular reason to use funcall here, instead of calling the
> functions literally?

Just to not repeat the string literal.  Could definitely be replaced
with a let binding.