bug#68018: [PATCH] Fix previous page and next page tooltips in doc-view.el

bug#68018: [PATCH] Fix previous page and next page tooltips in doc-view.el

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Tags: patch


In docview.el, the toolbar icons used to navigate to the previous and
next pages have incorrect tooltips.  I've attached a patch to address
this issue.  Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-previous-page-and-next-page-tooltips-in-doc-view.patch --]
[-- Type: text/patch, Size: 2499 bytes --]

From 0e3fd24ab6e6325c2239d3da690dd05fa9fa065c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <dmartin@Daniels-MacBook-Pro.local>
Date: Sun, 24 Dec 2023 23:58:36 +0100
Subject: [PATCH] Fix previous page and next page tooltips in doc-view.el

* lisp/doc-view.el (doc-view-tool-bar-map): Fix toolbar items to go to
next and previous pages in a document.
* etc/images/prev-page.pbm: Rename icon that moves to the previous page.
* etc/images/prev-page.xpm: The same for the icon in XPM format.
---
 etc/images/{last-page.pbm => prev-page.pbm} | Bin
 etc/images/{last-page.xpm => prev-page.xpm} |   0
 lisp/doc-view.el                            |   6 +++---
 3 files changed, 3 insertions(+), 3 deletions(-)
 rename etc/images/{last-page.pbm => prev-page.pbm} (100%)
 rename etc/images/{last-page.xpm => prev-page.xpm} (100%)

diff --git a/etc/images/last-page.pbm b/etc/images/prev-page.pbm
similarity index 100%
rename from etc/images/last-page.pbm
rename to etc/images/prev-page.pbm
diff --git a/etc/images/last-page.xpm b/etc/images/prev-page.xpm
similarity index 100%
rename from etc/images/last-page.xpm
rename to etc/images/prev-page.xpm
diff --git a/lisp/doc-view.el b/lisp/doc-view.el
index 2fdb49f3e42..d6063a39ef6 100644
--- a/lisp/doc-view.el
+++ b/lisp/doc-view.el
@@ -695,15 +695,15 @@ doc-view-tool-bar-map
                                    :enable 'doc-view--current-search-matches
                                    :help "Move to the next search result.")
     (define-key-after map [separator-2] menu-bar-separator)
-    (tool-bar-local-item-from-menu 'doc-view-previous-page "last-page"
+    (tool-bar-local-item-from-menu 'doc-view-previous-page "prev-page"
                                    map doc-view-mode-map :vert-only t
                                    :enable '(> (doc-view-current-page) 1)
-                                   :help "Move to the next page.")
+                                   :help "Move to the previous page.")
     (tool-bar-local-item-from-menu 'doc-view-next-page "next-page"
                                    map doc-view-mode-map :vert-only t
                                    :enable '(< (doc-view-current-page)
                                                (doc-view-last-page-number))
-                                   :help "Move to the last page.")
+                                   :help "Move to the next page.")
     map)
   "Like the default `tool-bar-map', but with additions for DocView.")
 
-- 
2.42.0

bug#68018: [PATCH] Fix previous page and next page tooltips in doc-view.el

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Daniel Martín <mardani29@yahoo.es> writes:

> In docview.el, the toolbar icons used to navigate to the previous and
> next pages have incorrect tooltips.  I've attached a patch to address
> this issue.  Thanks.

Thanks, but why rename the icon?

bug#68018: [PATCH] Fix previous page and next page tooltips in doc-view.el

[Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Po Lu <luangruo@yahoo.com> writes:

> Daniel Martín <mardani29@yahoo.es> writes:
>
>> In docview.el, the toolbar icons used to navigate to the previous and
>> next pages have incorrect tooltips.  I've attached a patch to address
>> this issue.  Thanks.
>
> Thanks, but why rename the icon?

So that the name of the icon matches what the icon really represents.
If you feel that renaming the icon may cause incompatibilities, here’s a
new patch that only changes the help tooltip.  Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-previous-page-and-next-page-tooltips-in-doc-view.patch --]
[-- Type: text/x-patch, Size: 1503 bytes --]

From bd5e5f563d7388ba71eb5496766bc1d5a5fdeb23 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <dmartin@Daniels-MacBook-Pro.local>
Date: Mon, 25 Dec 2023 11:43:39 +0100
Subject: [PATCH] Fix previous page and next page tooltips in doc-view.el

* lisp/doc-view.el (doc-view-tool-bar-map): Fix toolbar items to go to
next and previous pages in a document.
---
 lisp/doc-view.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/doc-view.el b/lisp/doc-view.el
index 2fdb49f3e42..c70b60d9f58 100644
--- a/lisp/doc-view.el
+++ b/lisp/doc-view.el
@@ -698,12 +698,12 @@ doc-view-tool-bar-map
     (tool-bar-local-item-from-menu 'doc-view-previous-page "last-page"
                                    map doc-view-mode-map :vert-only t
                                    :enable '(> (doc-view-current-page) 1)
-                                   :help "Move to the next page.")
+                                   :help "Move to the previous page.")
     (tool-bar-local-item-from-menu 'doc-view-next-page "next-page"
                                    map doc-view-mode-map :vert-only t
                                    :enable '(< (doc-view-current-page)
                                                (doc-view-last-page-number))
-                                   :help "Move to the last page.")
+                                   :help "Move to the next page.")
     map)
   "Like the default `tool-bar-map', but with additions for DocView.")
 
-- 
2.42.0

bug#68018: [PATCH] Fix previous page and next page tooltips in doc-view.el

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Daniel Martín <mardani29@yahoo.es> writes:

> So that the name of the icon matches what the icon really represents.

I don't see an appreciable distinction between "previous" and "last" in
this context, except perhaps that "previous" might refer to the page
previously visited (in contrast to the preceding page in the document),
which is not the function of the Doc View command in question.  As such,
please don't rename the icon.  Thanks.

bug#68018: [PATCH] Fix previous page and next page tooltips in doc-view.el

[Eli Zaretskii]

> Cc: 68018@debbugs.gnu.org
> Date: Mon, 25 Dec 2023 19:59:39 +0800
> From:  Po Lu via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Daniel Martín <mardani29@yahoo.es> writes:
> 
> > So that the name of the icon matches what the icon really represents.
> 
> I don't see an appreciable distinction between "previous" and "last" in
> this context, except perhaps that "previous" might refer to the page
> previously visited (in contrast to the preceding page in the document),
> which is not the function of the Doc View command in question.  As such,
> please don't rename the icon.  Thanks.

Doesn't the icon's name say what it is supposed to mean?  I don't
think I like to see "last-page" where the function is
doc-view-previous-page.  If the icon's image is not very
self-explanatory, we should perhaps look for a better icon, but that
is unrelated to this issue.

bug#68018: [PATCH] Fix previous page and next page tooltips in doc-view.el

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Doesn't the icon's name say what it is supposed to mean?  I don't

Yes.  The icon's name implies it will move one page backwards, in line
with the function of doc-view-previous-page.

Thanks.

bug#68018: [PATCH] Fix previous page and next page tooltips in doc-view.el

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: mardani29@yahoo.es,  68018@debbugs.gnu.org
> Date: Mon, 25 Dec 2023 20:59:01 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Doesn't the icon's name say what it is supposed to mean?  I don't
> 
> Yes.  The icon's name implies it will move one page backwards, in line
> with the function of doc-view-previous-page.

Then why is the icon called "LAST page" and not "PREVIOUS page"?

bug#68018: [PATCH] Fix previous page and next page tooltips in doc-view.el

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Po Lu <luangruo@yahoo.com>
>> Cc: mardani29@yahoo.es,  68018@debbugs.gnu.org
>> Date: Mon, 25 Dec 2023 20:59:01 +0800
>>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>> > Doesn't the icon's name say what it is supposed to mean?  I don't
>>
>> Yes.  The icon's name implies it will move one page backwards, in line
>> with the function of doc-view-previous-page.
>
> Then why is the icon called "LAST page" and not "PREVIOUS page"?

FWIW, I tend to agree that it's purpose would be be more clear if we
renamed it.  This icon was added in July 2023.

bug#68018: [PATCH] Fix previous page and next page tooltips in doc-view.el

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Then why is the icon called "LAST page" and not "PREVIOUS page"?

Because it was generated from "next-page.xpm", and the icon itself could
easily mean either "last in document" or "last in time".

bug#68018: [PATCH] Fix previous page and next page tooltips in doc-view.el

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: mardani29@yahoo.es,  68018@debbugs.gnu.org
> Date: Tue, 26 Dec 2023 10:02:21 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Then why is the icon called "LAST page" and not "PREVIOUS page"?
> 
> Because it was generated from "next-page.xpm", and the icon itself could
> easily mean either "last in document" or "last in time".

Then how about copying last-page.* into prev-page.*, so that we keep
both?

bug#68018: [PATCH] Fix previous page and next page tooltips in doc-view.el

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> Then how about copying last-page.* into prev-page.*, so that we keep
> both?

I think there are tasks worthier of our attention than duplicating an
icon in order to create a copy whose name reflects a marginally
different meaning from that which the original's does.  It would spare
us a lot of quibbling if we focused on the substantive part of this
change, namely the two corrections to the tooltips, rather than the
renaming of an icon.  But maybe that's just me, and we do want the names
of each icon in etc/images to be 150% representative of every meaning it
might carry.

TIA.

bug#68018: [PATCH] Fix previous page and next page tooltips in doc-view.el

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: mardani29@yahoo.es,  68018@debbugs.gnu.org
> Date: Wed, 27 Dec 2023 21:02:18 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Then how about copying last-page.* into prev-page.*, so that we keep
> > both?
> 
> I think there are tasks worthier of our attention than duplicating an
> icon in order to create a copy whose name reflects a marginally
> different meaning from that which the original's does.

You prefer to have to answer questions from surprised people about why
it's "last" and not "prev"?  Fine by me.

> It would spare us a lot of quibbling if we focused on the
> substantive part of this change, namely the two corrections to the
> tooltips, rather than the renaming of an icon.

Aren't those other parts no-brainers?

> But maybe that's just me, and we do want the names of each icon in
> etc/images to be 150% representative of every meaning it might
> carry.

Are we going to argue again about the importance of mnemonic names?

bug#68018: [PATCH] Fix previous page and next page tooltips in doc-view.el

[Eli Zaretskii]

> Cc: 68018@debbugs.gnu.org
> Date: Mon, 25 Dec 2023 11:54:07 +0100
> From:  Daniel Martín via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Po Lu <luangruo@yahoo.com> writes:
> 
> > Daniel Martín <mardani29@yahoo.es> writes:
> >
> >> In docview.el, the toolbar icons used to navigate to the previous and
> >> next pages have incorrect tooltips.  I've attached a patch to address
> >> this issue.  Thanks.
> >
> > Thanks, but why rename the icon?
> 
> So that the name of the icon matches what the icon really represents.
> If you feel that renaming the icon may cause incompatibilities, here’s a
> new patch that only changes the help tooltip.  Thanks.

Thanks, installed on master, and closing the bug.