Subject: Re: bug#37548: Implement sanitation of single-file package long description

From: Bruno Félix Rezende Ribeiro <[email-masked]>
Date: Tue, 08 Oct 2019 05:36:40 -0300

> Stefan Kangas <[email-masked]> writes:
>
> I noticed something else:  There is actually already code duplication
> in package.el -- there is code to strip the commentary section in both
> package--get-description and describe-package-1.
>
> Perhaps it would make sense to look this all over and see how we can do better?

I decided to tackle the problem’s root.  After figuring out that every
function depending on ‘lm-commentary’ implemented their own ad-hoc
sanitation for the same effect, I changed ‘lm-commentary’ to return a
sanitized string and removed the code/functionality duplication from all
callers.

> I also have two general questions, which are applicable to both your
> recent patches:
>
> 1. It looks likely that this together with your other patch and your
> previous contributions will together amount to more than 15 lines of
> code.  That means that you would have to sign Copyright Assignment
> papers for GNU Emacs.  I see you're emailing from gnu.org, so I assume
> there are no surprises for you here; I guess Eli can help you sort
> that out if it's not already.

I’ve assigned my copyright for work on Emacs to the FSF already.

> 2. Could you please provide a commit message formatted as a changelog
> entry?  Details on this are in the CONTRIBUTE file in the repository.

Please, find it in the patch attached.

>From d3e3983fc6cb74900bfa99f0bfcf2497ab396d67 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bruno=20F=C3=A9lix=20Rezende=20Ribeiro?= <[email-masked]>
Date: Tue, 8 Oct 2019 04:32:18 -0300
Subject: [PATCH] Globally sanitize single-file package long descriptions
 (Bug#37548)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Consistent with multi-file package descriptions which don’t have
commentary sections nor double semicolon prefixes.
* lisp/emacs-lisp/lisp-mnt.el (lm-commentary): Remove commentary
header, double semicolon prefixes of each line, trailing new-lines and
trailing white-space from commentary.
* lisp/emacs-lisp/package.el (package--get-description)
(describe-package-1):
* lisp/finder.el (finder-commentary):
* lisp/info.el (Info-finder-find-node): remove ad-hoc sanitation.
---
 lisp/emacs-lisp/lisp-mnt.el | 14 +++++++++++++-
 lisp/emacs-lisp/package.el  | 30 ++++++++----------------------
 lisp/finder.el              |  8 +-------
 lisp/info.el                | 16 ++--------------
 4 files changed, 24 insertions(+), 44 deletions(-)

diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
index 91c7615..dda7895 100644
--- a/lisp/emacs-lisp/lisp-mnt.el
+++ b/lisp/emacs-lisp/lisp-mnt.el
@@ -4,6 +4,7 @@
 ;; Inc.
 
 ;; Author: Eric S. Raymond <[email-masked]>
+;;         Bruno Félix Rezende Ribeiro <[email-masked]>
 ;; Maintainer: [email-masked]
 ;; Created: 14 Jul 1992
 ;; Keywords: docs
@@ -485,7 +486,18 @@ absent, return nil."
   (lm-with-file file
     (let ((start (lm-commentary-start)))
       (when start
-        (buffer-substring-no-properties start (lm-commentary-end))))))
+        (replace-regexp-in-string       ; Get rid of...
+         "[[:blank:]]*$" ""             ; trailing white-space
+         (replace-regexp-in-string
+          (format "%s\\|%s\\|%s"
+                  ;; commentary header
+                  (concat "^;;;[[:blank:]]*\\("
+                          lm-commentary-header
+                          "\\):[[:blank:]\n]*")
+                  "^;;[[:blank:]]*"     ; double semicolon prefix
+                  "[[:blank:]\n]*\\'")  ; trailing new-lines
+          "" (buffer-substring-no-properties
+              start (lm-commentary-end))))))))
 
 (defun lm-homepage (&optional file)
   "Return the homepage in file FILE, or current buffer if FILE is nil."
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ab1fb8b..f65559d 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -4,6 +4,7 @@
 
 ;; Author: Tom Tromey <[email-masked]>
 ;;         Daniel Hackney <[email-masked]>
+;;         Bruno Félix Rezende Ribeiro <[email-masked]>
 ;; Created: 10 Mar 2007
 ;; Version: 1.1.0
 ;; Keywords: tools
@@ -2349,18 +2350,9 @@ The description is read from the installed package files."
      result
 
      ;; Look for Commentary header.
-     (let ((mainsrcfile (expand-file-name (format "%s.el" (package-desc-name desc))
-                                          srcdir)))
-       (when (file-readable-p mainsrcfile)
-         (with-temp-buffer
-           (insert (or (lm-commentary mainsrcfile) ""))
-           (goto-char (point-min))
-           (when (re-search-forward "^;;; Commentary:\n" nil t)
-             (replace-match ""))
-           (while (re-search-forward "^\\(;+ ?\\)" nil t)
-             (replace-match ""))
-           (buffer-string))))
-     )))
+     (or (lm-commentary (expand-file-name
+                         (format "%s.el" (package-desc-name desc)) srcdir))
+         ""))))
 
 (defun describe-package-1 (pkg)
   "Insert the package description for PKG.
@@ -2555,16 +2547,10 @@ Helper function for `describe-package'."
       (if built-in
           ;; For built-in packages, get the description from the
           ;; Commentary header.
-          (let ((fn (locate-file (format "%s.el" name) load-path
-                                 load-file-rep-suffixes))
-                (opoint (point)))
-            (insert (or (lm-commentary fn) ""))
-            (save-excursion
-              (goto-char opoint)
-              (when (re-search-forward "^;;; Commentary:\n" nil t)
-                (replace-match ""))
-              (while (re-search-forward "^\\(;+ ?\\)" nil t)
-                (replace-match ""))))
+          (insert (or (lm-commentary (locate-file (format "%s.el" name)
+                                                  load-path
+                                                  load-file-rep-suffixes))
+                      ""))
 
         (if (package-installed-p desc)
             ;; For installed packages, get the description from the
diff --git a/lisp/finder.el b/lisp/finder.el
index 89706cf..02d25ec 100644
--- a/lisp/finder.el
+++ b/lisp/finder.el
@@ -4,6 +4,7 @@
 ;; Inc.
 
 ;; Author: Eric S. Raymond <[email-masked]>
+;;         Bruno Félix Rezende Ribeiro <[email-masked]>
 ;; Created: 16 Jun 1992
 ;; Version: 1.0
 ;; Keywords: help
@@ -394,13 +395,6 @@ FILE should be in a form suitable for passing to `locate-library'."
     (erase-buffer)
     (insert str)
     (goto-char (point-min))
-    (delete-blank-lines)
-    (goto-char (point-max))
-    (delete-blank-lines)
-    (goto-char (point-min))
-    (while (re-search-forward "^;+ ?" nil t)
-      (replace-match "" nil nil))
-    (goto-char (point-min))
     (while (re-search-forward "\\<\\([-[:alnum:]]+\\.el\\)\\>" nil t)
       (if (locate-library (match-string 1))
           (make-text-button (match-beginning 1) (match-end 1)
diff --git a/lisp/info.el b/lisp/info.el
index 02f3ea5..4262219 100644
--- a/lisp/info.el
+++ b/lisp/info.el
@@ -3780,20 +3780,8 @@ Build a menu of the possible matches."
     ;; there is no "nxml.el" (it's nxml-mode.el).
     ;; But package.el makes the same assumption.
     ;; I think nxml is the only exception - maybe it should be just be renamed.
-    (let ((str (ignore-errors (lm-commentary (find-library-name nodename)))))
-      (if (null str)
-	  (insert "Can’t find package description.\n\n")
-	(insert
-	 (with-temp-buffer
-	   (insert str)
-	   (goto-char (point-min))
-	   (delete-blank-lines)
-	   (goto-char (point-max))
-	   (delete-blank-lines)
-	   (goto-char (point-min))
-	   (while (re-search-forward "^;+ ?" nil t)
-	     (replace-match "" nil nil))
-	   (buffer-string))))))))
+    (insert (or (ignore-errors (lm-commentary (find-library-name nodename)))
+                (insert "Can’t find package description.\n\n"))))))
 
 ;;;###autoload
 (defun info-finder (&optional keywords)
-- 
2.7.4

-- 
Bruno Félix Rezende Ribeiro (oitofelix) [0x28D618AF]
<http://oitofelix.freeshell.org/>