diff mbox series

[bug#53548] Harden beautify-description

Message ID 20220202163731.4e3216f2@ens-lyon.fr
State Accepted
Headers show
Series [bug#53548] Harden beautify-description | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Alice BRENON Feb. 2, 2022, 3:37 p.m. UTC 
Hi !

Thank you again for the help. I think it looks really better with the
translated reference to the info page. I feared the issues I raised
would yield unending discussions but actually with your edit it's
already pretty neat while remaining very simple. That's inspiring.

For the others interested in this thread, I also made the import of G_
from (guix i18n) explicit because it was being re-exported from (guix
ui), and while I was at it, I made the only used import from (guix ui),
that is fill-paragraph, explicit.

Hope you like this second version better : )

Alice


Le Tue, 01 Feb 2022 00:09:40 +0100,
Ludovic Courtès <ludo@gnu.org> a écrit :

> Hi Alice,
> 
> Alice BRENON <alice.brenon@ens-lyon.fr> skribis:
> 
> > Missing metadata in packages used to break their imports, at least
> > for opam packages, until this was fixed by Julien in
> > 24aa7b3c21309b63cc6e8e18d6417d2cddccf6c6. When Xinglu improved the
> > output of descriptions in 155fc235b5e1b41b4665c782365dd2bf11beae9c,
> > it made the imports break again in the case when the `description`
> > field is missing and Julien's fix applies, returning #f in
> > `metadata-ref` instead of crashing. Trouble is:
> > beautify-description expects its description argument to be a
> > string and will crash if this assumption isn't met (when calling
> > the `string-prefix?` predicate).  
> 
> Thanks for explaining!  Some comments:
> 
> > From 22f0523ef3599b45af9448bd4f31f7b8f8ce6af2 Mon Sep 17 00:00:00
> > 2001 From: Alice BRENON <alice.brenon@ens-lyon.fr>
> > Date: Wed, 26 Jan 2022 09:27:12 +0100
> > Subject: [PATCH] guix: import: Harden beautify-description.
> >
> > * guix/import/utils.scm (beautify-description): Handle non-string
> > arguments.
> > * guix/import/opam.scm: [use-modules] Make imports explicit for
> > module (guix import utils).  
> 
> [...]
> 
> >    #:use-module ((guix utils) #:select (cache-directory  
> >                                         version>?  
> >                                         call-with-temporary-output-file))
> > -  #:use-module (guix import utils)
> > +  #:use-module ((guix import utils) #:select (beautify-description
> > +                                              guix-hash-url
> > +                                              recursive-import
> > +                                              spdx-string->license
> > +                                              url-fetch))  
> 
> It can’t hurt.
> 
> > +                  ((not (string? description))
> > +                   (let ((home-url "https://guix.gnu.org/")
> > +                         (doc-path "fr/manual/devel/en/html_node/")
> > +                         (page
> > +
> > "Synopses-and-Descriptions.html#Synopses-and-Descriptions"))
> > +                     (string-append
> > +                       "Please fill in the description of your
> > package before "
> > +                       "submitting ! See "
> > +                       home-url doc-path page)))  
> 
> I’d avoid the URL and maybe make the string translatable, like so:
> 
>   (G_ "This package lacks a description …
> Run \"info '(guix) Synopses and Descriptions'\" for more
> information.")
> 
> … where ‘G_’ comes from (guix i18n).
> 
> WDYT?
> 
> This looks like a welcome improvement to me.
> 
> Thanks!
> 
> Ludo’.
diff mbox series

Patch

From cfb57a40a90bfc31d1b846f3e981469285f1bd7b Mon Sep 17 00:00:00 2001
From: Alice BRENON <alice.brenon@ens-lyon.fr>
Date: Wed, 26 Jan 2022 09:27:12 +0100
Subject: [PATCH] guix: import: Harden beautify-description.

* guix/import/utils.scm (beautify-description): Handle non-string
arguments.
[use-modules]: Explicitly import G_ from (guix i18n) and make (guix ui)
import explicit.
* guix/import/opam.scm: [use-modules] Make imports explicit for module
(guix import utils).
---
 guix/import/opam.scm  | 8 ++++++--
 guix/import/utils.scm | 7 ++++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/guix/import/opam.scm b/guix/import/opam.scm
index a6f6fe8c9f..f569c921b1 100644
--- a/guix/import/opam.scm
+++ b/guix/import/opam.scm
@@ -3,7 +3,7 @@ 
 ;;; Copyright © 2020 Martin Becze <mjbecze@riseup.net>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
 ;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
-;;; Copyright © 2021 Alice Brenon <alice.brenon@ens-lyon.fr>
+;;; Copyright © 2021, 2022 Alice Brenon <alice.brenon@ens-lyon.fr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -42,7 +42,11 @@  (define-module (guix import opam)
   #:use-module ((guix utils) #:select (cache-directory
                                        version>?
                                        call-with-temporary-output-file))
-  #:use-module (guix import utils)
+  #:use-module ((guix import utils) #:select (beautify-description
+                                              guix-hash-url
+                                              recursive-import
+                                              spdx-string->license
+                                              url-fetch))
   #:use-module ((guix licenses) #:prefix license:)
   #:export (opam->guix-package
             opam-recursive-import
diff --git a/guix/import/utils.scm b/guix/import/utils.scm
index 1c3cfa3e0b..934b224bec 100644
--- a/guix/import/utils.scm
+++ b/guix/import/utils.scm
@@ -10,6 +10,7 @@ 
 ;;; Copyright © 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;; Copyright © 2021 Xinglu Chen <public@yoctocell.xyz>
+;;; Copyright © 2022 Alice Brenon <alice.brenon@ens-lyon.fr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -37,10 +38,11 @@  (define-module (guix import utils)
   #:use-module (guix discovery)
   #:use-module (guix build-system)
   #:use-module (guix gexp)
+  #:use-module ((guix i18n) #:select (G_))
   #:use-module (guix store)
   #:use-module (guix download)
   #:use-module (guix sets)
-  #:use-module (guix ui)
+  #:use-module ((guix ui) #:select (fill-paragraph))
   #:use-module (gnu packages)
   #:use-module (ice-9 match)
   #:use-module (ice-9 rdelim)
@@ -241,6 +243,9 @@  (define* (beautify-description description #:optional (length 80))
 a proper sentence and by using two spaces between sentences, and wrap lines at
 LENGTH characters."
   (let ((cleaned (cond
+                  ((not (string? description))
+                   (G_ "This package lacks a description.  Run \"info '(guix)
+                       Synopses and Descriptions'\" for more information."))
                   ((string-prefix? "A " description)
                    (string-append "This package provides a"
                                   (substring description 1)))
-- 
2.34.0