bug#50497: [PATCH] Adding eww-{next,previous,up,top}-path.

bug#50497: [PATCH] Adding eww-{next,previous,up,top}-path.

[Yuchen Pei]

[-- Attachment #1.1: Type: text/plain, Size: 714 bytes --]

I often find myself wanting to navigate paginated web pages 
(e.g. <https://media.libreplanet.org/videos?page=4>), or to go up 
or all the way up when visiting a web page, which is why I added 
these functions to my eww.

Does this change make sense?  If so I will amend the patch to 
include tests, doc and bug number.

Note that there seem to be currently no tests for eww, so I will 
need to create a new file.

BTW a newbie question: CONTRIBUTE mentions the commit message 
should include bug number, but when I send a patch like this a bug 
number is only generated afterwards.  Does this mean I should 
report a bug, wait for the item to be created in debbugs, before 
sending a patch for the bug?

Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Add-eww-next-previous-up-top-path.patch --]
[-- Type: text/x-patch, Size: 3054 bytes --]

From 8abc291f6fb9bd85c51fd1b0b7f6dce044b231a5 Mon Sep 17 00:00:00 2001
From: Yuchen Pei <hi@ypei.me>
Date: Fri, 10 Sep 2021 12:41:49 +1000
Subject: [PATCH] Add eww-{next,previous,up,top}-path.

Similar to eww-{next,previous,up,top}-url, but working based on the
url path rather than the rel attribute.

* lisp/net/eww.el (eww-next-path, eww-previous-path, eww-up-path,
eww-top-path).
* doc/misc/eww.texi TODO.
* test/lisp/net/eww-tests.el TODO.
---
 lisp/net/eww.el | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/lisp/net/eww.el b/lisp/net/eww.el
index 90301e92ac..ded01f97e8 100644
--- a/lisp/net/eww.el
+++ b/lisp/net/eww.el
@@ -974,6 +974,10 @@ eww-mode-map
     (define-key map "p" 'eww-previous-url)
     (define-key map "u" 'eww-up-url)
     (define-key map "t" 'eww-top-url)
+    (define-key map "N" 'eww-next-path)
+    (define-key map "P" 'eww-previous-path)
+    (define-key map "U" 'eww-up-path)
+    (define-key map "T" 'eww-top-path)
     (define-key map "&" 'eww-browse-with-external-browser)
     (define-key map "d" 'eww-download)
     (define-key map "w" 'eww-copy-page-url)
@@ -1194,6 +1198,50 @@ eww-top-url
 	(eww-browse-url (shr-expand-url best-url (plist-get eww-data :url)))
       (user-error "No `top' for this page"))))
 
+(defun eww-next-path ()
+  "Go to the `next' page according to the url path.
+The url of the `next' page is generated by incrementing the last
+number in the path."
+  (interactive)
+  (let ((url (plist-get eww-data :url)))
+    (when (string-match "^\\(.*?\\)\\([0-9]+\\)\\(.*\\)$" url)
+      (eww (concat
+	    (match-string 1 url)
+	    (number-to-string
+	     (1+ (string-to-number (match-string 2 url))))
+	    (match-string 3 url))))))
+
+(defun eww-previous-path ()
+  "Go to the `previous' page according to the url path.
+The url of the `previous' page is generated by decrementing the
+last integer in the path."
+  (interactive)
+  (let ((url (plist-get eww-data :url)))
+    (when (string-match "^\\(.*\\)\\([0-9]+\\)\\(.*\\)$" url)
+      (eww (concat
+	    (match-string 1 url)
+	    (number-to-string
+	     (1- (string-to-number (match-string 2 url))))
+	    (match-string 3 url))))))
+
+(defun eww-up-path ()
+  "Go to the `parent' page according to the url path.
+The url of the `parent' page is generated by removing the last
+segment delimited by a forward slash."
+  (interactive)
+  (let ((url (plist-get eww-data :url)))
+    (when (and (string-match "^\\(.*//.*/\\)[^/]+\\(/\\)?$" url)
+	       (match-string 1 url))
+      (eww (match-string 1 url)))))
+
+(defun eww-top-path ()
+  "Go to the domain."
+  (interactive)
+  (let ((url (plist-get eww-data :url)))
+    (when (and (string-match "^\\(.*//.*?/\\).*$" url)
+	       (match-string 1 url))
+      (eww (match-string 1 url)))))
+
 (defun eww-reload (&optional local encode)
   "Reload the current page.
 If LOCAL is non-nil (interactively, the command was invoked with
-- 
2.33.0


[-- Attachment #1.3: Type: text/plain, Size: 131 bytes --]


-- 
Best,
Yuchen

PGP Key: 47F9 D050 1E11 8879 9040  4941 2126 7E93 EF86 DFD0
           <https://ypei.me/assets/ypei-pubkey.txt>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 243 bytes --]

bug#50497: [PATCH] Adding eww-{next,previous,up,top}-path.

[Juri Linkov]

> I often find myself wanting to navigate paginated web pages
> (e.g. <https://media.libreplanet.org/videos?page=4>), or to go up or all
> the way up when visiting a web page, which is why I added these functions
> to my eww.
>
> Does this change make sense?

This reminds a very useful Firefox add-on "Go Up" that lets you go up
a level to the parent directory on the current website by pressing
Alt+UpArrow, and some other add-ons I don't remember their names.

> +    (define-key map "N" 'eww-next-path)
> +    (define-key map "P" 'eww-previous-path)
> +    (define-key map "U" 'eww-up-path)
> +    (define-key map "T" 'eww-top-path)

Would it be possible to combine this feature with the existing

    (define-key map "n" 'eww-next-url)
    (define-key map "p" 'eww-previous-url)
    (define-key map "u" 'eww-up-url)
    (define-key map "t" 'eww-top-url)

that rely on special attributes.  I mean to install your new commands.
Then later add fallbacks to both sets of commands: if there is
no special attribute, then "n"/"p"/"u"/"t" could try to deduce the
page from URL by calling your new commands, instead of signaling
an error as they do currently by (user-error "No `next' on this page").
And vice versa: if your commands don't match a number in the URL, then
fall back to "n"/"p"/"u"/"t", maybe optionally.

bug#50497: [PATCH] Adding eww-{next,previous,up,top}-path.

[Yuchen Pei]

[-- Attachment #1: Type: text/plain, Size: 1744 bytes --]


Juri Linkov <juri@linkov.net> writes:

>> I often find myself wanting to navigate paginated web pages
>> (e.g. <https://media.libreplanet.org/videos?page=4>), or to go 
>> up or all
>> the way up when visiting a web page, which is why I added these 
>> functions
>> to my eww.
>>
>> Does this change make sense?
>
> This reminds a very useful Firefox add-on "Go Up" that lets you 
> go up
> a level to the parent directory on the current website by 
> pressing
> Alt+UpArrow, and some other add-ons I don't remember their 
> names.
>
>> +    (define-key map "N" 'eww-next-path)
>> +    (define-key map "P" 'eww-previous-path)
>> +    (define-key map "U" 'eww-up-path)
>> +    (define-key map "T" 'eww-top-path)
>
> Would it be possible to combine this feature with the existing
>
>     (define-key map "n" 'eww-next-url)
>     (define-key map "p" 'eww-previous-url)
>     (define-key map "u" 'eww-up-url)
>     (define-key map "t" 'eww-top-url)
>
> that rely on special attributes.  I mean to install your new 
> commands.
> Then later add fallbacks to both sets of commands: if there is
> no special attribute, then "n"/"p"/"u"/"t" could try to deduce 
> the
> page from URL by calling your new commands, instead of signaling
> an error as they do currently by (user-error "No `next' on this 
> page").

This sounds good to me, if it does not break people's workflows.

> And vice versa: if your commands don't match a number in the 
> URL, then
> fall back to "n"/"p"/"u"/"t", maybe optionally.

I think the previous fallback should be sufficient, and I feel a 
"mutual" fallback like this could be confusing.

-- 
Best,
Yuchen

PGP Key: 47F9 D050 1E11 8879 9040  4941 2126 7E93 EF86 DFD0
           <https://ypei.me/assets/ypei-pubkey.txt>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 243 bytes --]

bug#50497: [PATCH] Adding eww-{next,previous,up,top}-path.

[Lars Ingebrigtsen]

Yuchen Pei <hi@ypei.me> writes:

> I often find myself wanting to navigate paginated web pages
> (e.g. <https://media.libreplanet.org/videos?page=4>), or to go up or
> all the way up when visiting a web page, which is why I added these
> functions to my eww.
>
> Does this change make sense?  If so I will amend the patch to include
> tests, doc and bug number.

I'm not sure this is general enough to include in eww -- these are
commands that will fail in 99.7% of all web pages, because they don't
have URLs that are structured this way.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#50497: [PATCH] Adding eww-{next,previous,up,top}-path.

[Yuchen Pei]

[-- Attachment #1: Type: text/plain, Size: 1039 bytes --]


Lars Ingebrigtsen <larsi@gnus.org> writes:

> Yuchen Pei <hi@ypei.me> writes:
>
>> I often find myself wanting to navigate paginated web pages
>> (e.g. <https://media.libreplanet.org/videos?page=4>), or to go 
>> up or
>> all the way up when visiting a web page, which is why I added 
>> these
>> functions to my eww.
>>
>> Does this change make sense?  If so I will amend the patch to 
>> include
>> tests, doc and bug number.
>
> I'm not sure this is general enough to include in eww -- these 
> are
> commands that will fail in 99.7% of all web pages, because they 
> don't
> have URLs that are structured this way.

OK, how about having them as a fallback to 
eww-{nex,previous,up,top}-url, as Juri mentioned?  From my 
experience these -url functions don't work for most web pages 
either, so adding my functions as a fallback would increase the 
chance that the keys will work in some meaningful way.

-- 
Best,
Yuchen

PGP Key: 47F9 D050 1E11 8879 9040  4941 2126 7E93 EF86 DFD0
           <https://ypei.me/assets/ypei-pubkey.txt>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 243 bytes --]

bug#50497: [PATCH] Adding eww-{next,previous,up,top}-path.

[Juri Linkov]

>> I often find myself wanting to navigate paginated web pages
>> (e.g. <https://media.libreplanet.org/videos?page=4>), or to go up or
>> all the way up when visiting a web page, which is why I added these
>> functions to my eww.
>>
>> Does this change make sense?  If so I will amend the patch to include
>> tests, doc and bug number.
>
> I'm not sure this is general enough to include in eww -- these are
> commands that will fail in 99.7% of all web pages, because they don't
> have URLs that are structured this way.

Actually, the same 99.7% of all web pages doesn't provide special
attributes "next"/"previous"/"up" used by 'n'/'p'/'u' eww commands.
So using URL regexp matching as a fallback for other 99.7% of web pages
that don't have numbers in URL will support (100% - 99.7%) * 2 pages.

bug#50497: [PATCH] Adding eww-{next,previous,up,top}-path.

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> Actually, the same 99.7% of all web pages doesn't provide special
> attributes "next"/"previous"/"up" used by 'n'/'p'/'u' eww commands.

That's also true, so I was sceptical about adding that (because it also
makes `C-s' etc behave sub-optimally in 99.7% of web pages), so I've
been pondering whether to remove it (or at least hide it behind and
option defaulting to "off").  But at least it has one thing going for
it -- it's a standard mechanism, not using guesswork.

> So using URL regexp matching as a fallback for other 99.7% of web pages
> that don't have numbers in URL will support (100% - 99.7%) * 2 pages.

It's true, but it'll also make `C-s' (at the end of an eww buffer) guess
at what's the next buffer, load it, and search through that instead.
Which is just kinda problematic, since an URL with a number in it isn't
necessarily related to the URL with the next number in it.

We'd be displaying web pages to the user that the user hasn't asked for,
and that the person who wrote the web site didn't intend for the user to
navigate in that way.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#50497: [PATCH] Adding eww-{next,previous,up,top}-path.

[Lars Ingebrigtsen]

Yuchen Pei <hi@ypei.me> writes:

> OK, how about having them as a fallback to
> eww-{nex,previous,up,top}-url, as Juri mentioned?  From my experience
> these -url functions don't work for most web pages either, so adding
> my functions as a fallback would increase the chance that the keys
> will work in some meaningful way.

See my response to Juri.  For the explicit commands (i.e., not involved
in isearch) it might make sense, because the user actually asked to see
"next page".  I think it's a DWIM step too far, though.  Just because
you've on https://foo.bar/?4524 doesn't mean that the site author had
meant for visitors to that page to go to https://foo.bar/?4525, which
may be a totally different topic (for instance, something Not Safe For
Work).

The user is free to edit the URLs as they wish, but as a command in a
web browser?  I'm sceptical that we'd be doing the users a favour.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#50497: [PATCH] Adding eww-{next,previous,up,top}-path.

[Juri Linkov]

>> Actually, the same 99.7% of all web pages doesn't provide special
>> attributes "next"/"previous"/"up" used by 'n'/'p'/'u' eww commands.
>
> That's also true, so I was sceptical about adding that (because it also
> makes `C-s' etc behave sub-optimally in 99.7% of web pages), so I've
> been pondering whether to remove it (or at least hide it behind and
> option defaulting to "off").

But users like it:
https://www.reddit.com/r/emacs/comments/9oi1e3/ewws_awesome_isearch_support_just_blew_my_mind/

And if it behaves sub-optimally, it could be fixed by just creating
a new bug report :-)

> But at least it has one thing going for it -- it's a standard mechanism,
> not using guesswork.
>
>> So using URL regexp matching as a fallback for other 99.7% of web pages
>> that don't have numbers in URL will support (100% - 99.7%) * 2 pages.
>
> It's true, but it'll also make `C-s' (at the end of an eww buffer) guess
> at what's the next buffer, load it, and search through that instead.
> Which is just kinda problematic, since an URL with a number in it isn't
> necessarily related to the URL with the next number in it.
>
> We'd be displaying web pages to the user that the user hasn't asked for,
> and that the person who wrote the web site didn't intend for the user to
> navigate in that way.

I agree, then such fuzzy guesswork should be optional.

bug#50497: [PATCH] Adding eww-{next,previous,up,top}-path.

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

>> That's also true, so I was sceptical about adding that (because it also
>> makes `C-s' etc behave sub-optimally in 99.7% of web pages), so I've
>> been pondering whether to remove it (or at least hide it behind and
>> option defaulting to "off").
>
> But users like it:
> https://www.reddit.com/r/emacs/comments/9oi1e3/ewws_awesome_isearch_support_just_blew_my_mind/

Yeah, it's a neat trick (which makes people go "ooo").  So disabling it
would be a shame.  But it does make the user experience slightly worse
most of the time...

Actually, we could just tweak it -- today it says "repeat for next
buffer" even if there's no next buffer (it only checks afterwards),
apparently.  Hm...  Right, if eww only sets
`multi-isearch-next-buffer-function' when there's a rel=next/prev, then
this awkwardness disappears.

Now done.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#50497: [PATCH] Adding eww-{next,previous,up,top}-path.

[Juri Linkov]

> Juri Linkov <juri@linkov.net> writes:
>
>>> That's also true, so I was sceptical about adding that (because it also
>>> makes `C-s' etc behave sub-optimally in 99.7% of web pages), so I've
>>> been pondering whether to remove it (or at least hide it behind and
>>> option defaulting to "off").
>>
>> But users like it:
>> https://www.reddit.com/r/emacs/comments/9oi1e3/ewws_awesome_isearch_support_just_blew_my_mind/
>
> Yeah, it's a neat trick (which makes people go "ooo").  So disabling it
> would be a shame.  But it does make the user experience slightly worse
> most of the time...
>
> Actually, we could just tweak it -- today it says "repeat for next
> buffer" even if there's no next buffer (it only checks afterwards),
> apparently.  Hm...  Right, if eww only sets
> `multi-isearch-next-buffer-function' when there's a rel=next/prev, then
> this awkwardness disappears.
>
> Now done.

Thanks, this makes perfect sense - when there are no next/prev links,
then normal isearch should be used.

bug#50497: [PATCH] Adding eww-{next,previous,up,top}-path.

[Juri Linkov]

>>> That's also true, so I was sceptical about adding that (because it also
>>> makes `C-s' etc behave sub-optimally in 99.7% of web pages), so I've
>>> been pondering whether to remove it (or at least hide it behind and
>>> option defaulting to "off").
>>
>> But users like it:
>> https://www.reddit.com/r/emacs/comments/9oi1e3/ewws_awesome_isearch_support_just_blew_my_mind/
>
> Yeah, it's a neat trick (which makes people go "ooo").  So disabling it
> would be a shame.  But it does make the user experience slightly worse
> most of the time...
>
> Actually, we could just tweak it -- today it says "repeat for next
> buffer" even if there's no next buffer (it only checks afterwards),
> apparently.  Hm...  Right, if eww only sets
> `multi-isearch-next-buffer-function' when there's a rel=next/prev, then
> this awkwardness disappears.

But still I'd like to make `C-s' less sub-optimal when used
on pages with a rel=next/prev and to fix such complaints:

  "It turns out that this interacts badly with asynchronous loading of
  web pages -- I think the buffer is being searched before the web content
  is loaded, but with local content it's either all managing to happen
  fast enough that the issue doesn't arise, or it's being done in a more
  synchronous manner.  Hopefully this can be addressed, but in the
  meantime you may need to test with local html files."

What do you think about supporting synchronous mode in eww?
When adding a variable that causes eww-retrieve to use
url-retrieve-synchronously, isearch part could look like this:

  (defun eww-isearch-next-buffer (&optional _buffer wrap)
    (let ((eww-synchronous t))
      (if wrap
          (condition-case nil
              (eww-top-url)
            (error nil))
        (if isearch-forward
            (eww-next-url)
          (eww-previous-url))))
    (current-buffer))

bug#50497: [PATCH] Adding eww-{next,previous,up,top}-path.

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> What do you think about supporting synchronous mode in eww?
> When adding a variable that causes eww-retrieve to use
> url-retrieve-synchronously, isearch part could look like this:
>
>   (defun eww-isearch-next-buffer (&optional _buffer wrap)
>     (let ((eww-synchronous t))
>       (if wrap
>           (condition-case nil
>               (eww-top-url)
>             (error nil))
>         (if isearch-forward
>             (eww-next-url)
>           (eww-previous-url))))
>     (current-buffer))

Sure, makes sense to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#50497: [PATCH] Adding eww-{next,previous,up,top}-path.

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> The user is free to edit the URLs as they wish, but as a command in a
> web browser?  I'm sceptical that we'd be doing the users a favour.

So I'm closing this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no