GNU bug report logs - #37548
Implement sanitation of single-file package long description

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 37548 in the body.
You can then email your comments to 37548 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Bruno Félix Rezende Ribeiro <oitofelix <at> gnu.org>
To: bug-gnu-emacs <at> gnu.org
Subject: Implement sanitation of single-file package long description
Date: Sun, 29 Sep 2019 02:42:54 -0300

[Message part 1 (text/plain, inline)]

Hello Emacs developers,

The inlined patch implements sanitation of single-file package’s long
description which is derived from the package’s commentary header
section.  It removes the commentary header, the double semicolon prefix
of each line, trailing new-lines and trailing white-space.  I think this
is the usual practice for packages in GNU ELPA and MELPA repositories.
Furthermore it’s aligned with the intended behavior for multi-file
packages which is to read the long description from a README file[1] ---
which presumably does not have commentary sections nor double semicolon
prefixes.


Please, let me know of any changes required.
Thanks!


PS: For some reason I was not able to use a single regexp within a
single invocation of ‘replace-regexp-in-string’, as would be natural.
It simply didn’t work as expected.  It’s working fine now with nested
calls.

[package-x-single-file-package-long-description-sanitization.patch (text/x-diff, inline)]

diff --git a/lisp/emacs-lisp/package-x.el b/lisp/emacs-lisp/package-x.el
index 2815be3..7fe6f6d 100644
--- a/lisp/emacs-lisp/package-x.el
+++ b/lisp/emacs-lisp/package-x.el
@@ -159,6 +159,7 @@ DESCRIPTION is the text of the news item."
 
 (declare-function lm-commentary "lisp-mnt" (&optional file))
 (defvar tar-data-buffer)
+(defvar lm-commentary-header)
 
 (defun package-upload-buffer-internal (pkg-desc extension &optional archive-url)
   "Upload a package whose contents are in the current buffer.
@@ -204,7 +205,17 @@ if it exists."
 	       (split-version (package-desc-version pkg-desc))
 	       (commentary
                 (pcase file-type
-                  ('single (lm-commentary))
+                  ('single (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
+                             "" (lm-commentary))))
                   ('tar nil))) ;; FIXME: Get it from the README file.
                (extras (package-desc-extras pkg-desc))
 	       (pkg-version (package-version-join split-version))

[Message part 3 (text/plain, inline)]

Footnotes: 
[1]  I’ve implemented that in bug#37546.

-- 
 88888  FFFFF Bruno Félix Rezende Ribeiro (oitofelix) [0x28D618AF]
 8   8  F     http://oitofelix.freeshell.org/
 88888  FFFF  mailto:oitofelix <at> gnu.org
 8   8  F     irc://chat.freenode.org/oitofelix
 88888  F     xmpp://oitofelix <at> riseup.net

Message #8 received at 37548 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Bruno Félix Rezende Ribeiro <oitofelix <at> gnu.org>
Cc: 37548 <at> debbugs.gnu.org
Subject: Re: bug#37548: Implement sanitation of single-file package long
 description
Date: Mon, 30 Sep 2019 19:27:14 +0200

Bruno Félix Rezende Ribeiro <oitofelix <at> gnu.org> writes:

> Hello Emacs developers,

Hi Bruno,

And thanks for your patch.

> The inlined patch implements sanitation of single-file package’s long
> description which is derived from the package’s commentary header
> section.  It removes the commentary header, the double semicolon prefix
> of each line, trailing new-lines and trailing white-space.  I think this
> is the usual practice for packages in GNU ELPA and MELPA repositories.
> Furthermore it’s aligned with the intended behavior for multi-file
> packages which is to read the long description from a README file[1] ---
> which presumably does not have commentary sections nor double semicolon
> prefixes.

I agree with the change.  However, there seems to be code duplication
here, since the same is done in package.el:

          ;; 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 ""))))

Maybe it would make more sense to create a new function in package.el
that takes care of this?  That way we don't have the same
functionality in two places.

FWIW, I would probably prefer to base it on the code already in
package.el, since I find it a bit easier to read when the regular
expressions are split up.

Best regards,
Stefan Kangas

Message #11 received at 37548 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Bruno Félix Rezende Ribeiro <oitofelix <at> gnu.org>
Cc: 37548 <at> debbugs.gnu.org
Subject: Re: bug#37548: Implement sanitation of single-file package long
 description
Date: Mon, 30 Sep 2019 19:39:26 +0200

Stefan Kangas <stefan <at> marxist.se> writes:

> Maybe it would make more sense to create a new function in package.el
> that takes care of this?  That way we don't have the same
> functionality in two places.

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 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.

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

Best regards,
Stefan Kangas

Message #14 received at 37548 <at> debbugs.gnu.org (full text, mbox):

From: Bruno Félix Rezende Ribeiro <oitofelix <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: Bruno Félix Rezende Ribeiro <oitofelix <at> gnu.org>,
 37548 <at> debbugs.gnu.org
Subject: Re: bug#37548: Implement sanitation of single-file package long
 description
Date: Tue, 08 Oct 2019 05:36:40 -0300

[Message part 1 (text/plain, inline)]

> Stefan Kangas <stefan <at> marxist.se> 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.

[0001-Globally-sanitize-single-file-package-long-descripti.patch (text/x-diff, attachment)]

[Message part 3 (text/plain, inline)]

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

Message #17 received at 37548 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Bruno Félix Rezende Ribeiro <oitofelix <at> gnu.org>
Cc: 37548 <at> debbugs.gnu.org, oitofelix <at> gnu.org, stefan <at> marxist.se
Subject: Re: bug#37548: Implement sanitation of single-file package long
 description
Date: Tue, 08 Oct 2019 11:40:52 +0300

> From: Bruno Félix Rezende Ribeiro <oitofelix <at> gnu.org>
> Date: Tue, 08 Oct 2019 05:36:40 -0300
> Cc: Bruno Félix Rezende Ribeiro <oitofelix <at> gnu.org>,
>  37548 <at> debbugs.gnu.org
> 
> > 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.

Right, Bruno's copyright assignment is on file.

Message #22 received at 37548 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Bruno Félix Rezende Ribeiro <oitofelix <at> gnu.org>
Cc: 37548 <at> debbugs.gnu.org
Subject: Re: bug#37548: Implement sanitation of single-file package long
 description
Date: Mon, 11 Nov 2019 20:02:04 +0100

Bruno Félix Rezende Ribeiro <oitofelix <at> gnu.org> writes:

>> Stefan Kangas <stefan <at> marxist.se> 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.

Sorry for the late reply here.  I think your approach makes sense.

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

Great, thanks.

> Please, find it in the patch attached.

I think the patch looks good, but I didn't test it yet.

By the way, it would be very good if you would like to add tests.  I
don't think a lack of tests should stop us from applying your patch.
But it would be a big plus to have them.

> --- a/lisp/emacs-lisp/lisp-mnt.el
> +++ b/lisp/emacs-lisp/lisp-mnt.el
> @@ -4,6 +4,7 @@
>  ;; Inc.
>  
>  ;; Author: Eric S. Raymond <esr <at> snark.thyrsus.com>
> +;;         Bruno Félix Rezende Ribeiro <oitofelix <at> gnu.org>
>  ;; Maintainer: emacs-devel <at> gnu.org
>  ;; Created: 14 Jul 1992
>  ;; Keywords: docs

> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -4,6 +4,7 @@
>  
>  ;; Author: Tom Tromey <tromey <at> redhat.com>
>  ;;         Daniel Hackney <dan <at> haxney.org>
> +;;         Bruno Félix Rezende Ribeiro <oitofelix <at> gnu.org>
>  ;; Created: 10 Mar 2007
>  ;; Version: 1.1.0
>  ;; Keywords: tools

I think we don't usually add our names as authors in every file we
change.  We have other ways to track that, such as the AUTHORS file.

For this file, for instance, I see only one person in the author
field, but AFAICT there are 16 contributors with 99 commits in total.

Does anyone know if there is a general guideline for when to add your
name to the "Author" line at the top of the file?

Best regards,
Stefan Kangas

Message #25 received at 37548 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: oitofelix <at> gnu.org, 37548 <at> debbugs.gnu.org
Subject: Re: bug#37548: Implement sanitation of single-file package long
 description
Date: Thu, 14 Nov 2019 13:28:56 +0200

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Mon, 11 Nov 2019 20:02:04 +0100
> Cc: 37548 <at> debbugs.gnu.org
> 
> Does anyone know if there is a general guideline for when to add your
> name to the "Author" line at the top of the file?

Only when the file is first written, AFAIK.

Message #28 received at 37548 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Bruno Félix Rezende Ribeiro <oitofelix <at> gnu.org>
Cc: 37548 <at> debbugs.gnu.org
Subject: Re: bug#37548: Implement sanitation of single-file package long
 description
Date: Thu, 23 Jan 2020 21:56:53 +0100

close 37548 28.1
thanks

Stefan Kangas <stefan <at> marxist.se> writes:

>> Please, find it in the patch attached.
>
> I think the patch looks good, but I didn't test it yet.

Sorry for the long delay here.

I have now reviewed and tested your patch again, and also tested all
relevant functionality AFAICT.  I've pushed it to the master branch
with one or two minor stylistic changes, modulo this:

> I think we don't usually add our names as authors in every file we
> change.  We have other ways to track that, such as the AUTHORS file.

I'm consequently closing this bug.  Thank you again for your
contribution to Emacs.

Best regards,
Stefan Kangas

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.