bug#68970: [PATCH] In Info-url-alist, add .html extension to %e format-sequence

[Mekeor Melire <mekeor@posteo.de>]

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

2024-02-09 09:02 eliz@gnu.org:

> Why should %e omit the .html extension in the first place?

Because by omitting the .html extension, URLs are a little shorter. By
five characters.

> What problems will we cause if we make %e include the .html extension
> when that is required, and omit it when it isn't (like when converting
> the "Top" node)?

Yes, we could make %e include the .html extension for non-Top-nodes and
for Top-nodes it could be replaced with the empty string "".

> AFAICS, the use of %e in Emacs 29

Info-url-alist is not present in Emacs 29. It is only part of the
master-branch, i.e. upcoming Emacs 30. It was committed on 27th January
2024, i.e. just a couple of days ago, in a commit authored by me.

> doesn't add the .html extension to the produced URLs, but would
> producing the .html extension from %e cause any problems with the GNU
> manuals we had in Info-url-alist in Emacs 29?

No, adding the .html extension for non-Top-nodes to %e would not cause
any problems.

> Do we know of any package which augments Info-url-alist with specs
> that use %e.html?

It's very unlikely because it's just a couple of days old. I myself am
working on a package that will provide a community-maintained value for
Info-url-alist. While working on it, I realized that the current
implementation of %e would not work for many cases.

> I'm asking why cannot we make %e behave like your proposed %E?

We can do so. I was just trying to keep URLs a little shorter, by saving
five characters (".html"). But I think, you are right, it's more
important to cover as many cases as possible with as few
format-sequences as possible. Find attached a patch that implements
this. Note that in the mean time, the fill-column has been changed for
the Emacs repository via directory-local variables. For this patch, I
did not use the "new" fill-column value so that the changes to the
docstring can be seen well in the diff. Please let me know if you'd like
the docstrings to be refilled according to the new fill-column.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-In-Info-url-alist-add-.html-extension-to-e-format-se.patch --]
[-- Type: text/x-patch, Size: 4789 bytes --]

From 7eab913663fe8570d44f9c4399eb0547f1cc510f Mon Sep 17 00:00:00 2001
From: Mekeor Melire <mekeor@posteo.de>
Date: Fri, 9 Feb 2024 23:30:52 +0100
Subject: [PATCH] In Info-url-alist, add .html extension to %e format-sequence

* lisp/info.el (Info-url-for-node): Implement the change. (Bug#68970)
(Info-url-alist): Document the change.
* test/lisp/info-tests.el (test-info-urls): Adjust tests to account
for the change.
---
 lisp/info.el            | 31 +++++++++++++++++--------------
 test/lisp/info-tests.el | 12 ++++++------
 2 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/lisp/info.el b/lisp/info.el
index e91cc7b8e54..9be92f198c4 100644
--- a/lisp/info.el
+++ b/lisp/info.el
@@ -231,8 +231,9 @@ Info-url-alist
 MANUALs represents the name of one or more manuals.  It can
 either be a string or a list of strings.  URL-SPEC can be a
 string in which the substring \"%m\" will be expanded to the
-manual-name, \"%n\" to the node-name, and \"%e\" to the
-URL-encoded node-name (without a `.html' suffix).  (The
+manual-name and \"%n\" to the node-name. \"%e\" will expand to
+the URL-encoded node-name, including the `.html' extension; in
+case of the Top node, it will expand to the empty string.  (The
 URL-encoding of the node-name mimics GNU Texinfo, as documented
 at Info node `(texinfo)HTML Xref Node Name Expansion'.)
 Alternatively, URL-SPEC can be a function which is given
@@ -1924,18 +1925,20 @@ Info-url-for-node
                ;; (info "(texinfo) HTML Xref Node Name Expansion")
                (if (equal node "Top")
                  ""
-                 (url-hexify-string
-                   (string-replace " " "-"
-                     (mapconcat
-                       (lambda (ch)
-                         (if (or (< ch 32)      ; ^@^A-^Z^[^\^]^^^-
-                               (<= 33 ch 47)    ; !"#$%&'()*+,-./
-                               (<= 58 ch 64)    ; :;<=>?@
-                               (<= 91 ch 96)    ; [\]_`
-                               (<= 123 ch 127)) ; {|}~ DEL
-                           (format "_00%x" ch)
-                           (char-to-string ch)))
-                       node ""))))))
+                 (concat
+                   (url-hexify-string
+                     (string-replace " " "-"
+                       (mapconcat
+                         (lambda (ch)
+                           (if (or (< ch 32)      ; ^@^A-^Z^[^\^]^^^-
+                                 (<= 33 ch 47)    ; !"#$%&'()*+,-./
+                                 (<= 58 ch 64)    ; :;<=>?@
+                                 (<= 91 ch 96)    ; [\]_`
+                                 (<= 123 ch 127)) ; {|}~ DEL
+                             (format "_00%x" ch)
+                             (char-to-string ch)))
+                         node "")))
+                   ".html"))))
     (cond
       ((stringp url-spec)
         (format-spec url-spec
diff --git a/test/lisp/info-tests.el b/test/lisp/info-tests.el
index 0dfdbf417e8..9835491912d 100644
--- a/test/lisp/info-tests.el
+++ b/test/lisp/info-tests.el
@@ -29,17 +29,17 @@
 
 (ert-deftest test-info-urls ()
   (should (equal (Info-url-for-node "(emacs)Minibuffer")
-                 "https://www.gnu.org/software/emacs/manual/html_node/emacs/Minibuffer"))
+                 "https://www.gnu.org/software/emacs/manual/html_node/emacs/Minibuffer.html"))
   (should (equal (Info-url-for-node "(emacs)Minibuffer File")
-                 "https://www.gnu.org/software/emacs/manual/html_node/emacs/Minibuffer-File"))
+                 "https://www.gnu.org/software/emacs/manual/html_node/emacs/Minibuffer-File.html"))
   (should (equal (Info-url-for-node "(elisp)Backups and Auto-Saving")
-                 "https://www.gnu.org/software/emacs/manual/html_node/elisp/Backups-and-Auto_002dSaving"))
+                 "https://www.gnu.org/software/emacs/manual/html_node/elisp/Backups-and-Auto_002dSaving.html"))
   (should (equal (Info-url-for-node "(eintr)car & cdr")
-                 "https://www.gnu.org/software/emacs/manual/html_node/eintr/car-_0026-cdr"))
+                 "https://www.gnu.org/software/emacs/manual/html_node/eintr/car-_0026-cdr.html"))
   (should (equal (Info-url-for-node "(emacs-mime)\tIndex")
-                 "https://www.gnu.org/software/emacs/manual/html_node/emacs-mime/Index"))
+                 "https://www.gnu.org/software/emacs/manual/html_node/emacs-mime/Index.html"))
   (should (equal (Info-url-for-node  "(gnus) Don't Panic")
-                 "https://www.gnu.org/software/emacs/manual/html_node/gnus/Don_0027t-Panic"))
+                 "https://www.gnu.org/software/emacs/manual/html_node/gnus/Don_0027t-Panic.html"))
   (should-error (Info-url-for-node "(nonexistent)Example")))
 
 ;;; info-tests.el ends here
-- 
2.41.0