l...@gnu.org (Ludovic Courtès) writes:
> Mathieu Lirzin <m...@openmailbox.org> skribis:
>
>> diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
>> index b0427f7..0adb3bf 100644
>> --- a/guix/scripts/lint.scm
>> +++ b/guix/scripts/lint.scm
>> @@ -146,11 +146,11 @@ monad."
>> (define (check-texinfo-markup description)
>> "Check that DESCRIPTION can be parsed as a Texinfo fragment. If the
>> markup is valid return a plain-text version of DESCRIPTION, otherwise #f."
>> - (catch 'parser-error
>> - (lambda () (texi->plain-text description))
>> - (lambda (keys . args)
>> - (emit-warning package (_ "Texinfo markup in description is
>> invalid"))
>> - #f)))
>> + (unless (false-if-exception (texi->plain-text description))
>> + (emit-warning package
>> + (_ "Texinfo markup in description is invalid")
>> + 'description)
>> + #f))
>
> In general, it’s best to avoid ‘false-if-exception’ because it’s too
> coarse-grain. Here it’s probably OK though, because we want to catch
> any error that may occur in during the conversion. So this patch is OK
> (with appropriate commit log.)
Ok, thanks for the explanation.
>From 429ff285609120c4135eb64f1d6911924a24f5e6 Mon Sep 17 00:00:00 2001
From: Mathieu Lirzin <m...@openmailbox.org>
Date: Fri, 25 Sep 2015 00:37:36 +0200
Subject: [PATCH] lint: Improve 'check-texinfo-markup'.
* guix/scripts/lint.scm (check-description-style): Set 'field' parameter
when emitting a warning in 'check-texinfo-markup'. Catch any error
that may occur in during the 'texi->plain-text' conversion. This is a
followup to commit 2748ee3.
---
guix/scripts/lint.scm | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index b0427f7..0adb3bf 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -146,11 +146,11 @@ monad."
(define (check-texinfo-markup description)
"Check that DESCRIPTION can be parsed as a Texinfo fragment. If the
markup is valid return a plain-text version of DESCRIPTION, otherwise #f."
- (catch 'parser-error
- (lambda () (texi->plain-text description))
- (lambda (keys . args)
- (emit-warning package (_ "Texinfo markup in description is invalid"))
- #f)))
+ (unless (false-if-exception (texi->plain-text description))
+ (emit-warning package
+ (_ "Texinfo markup in description is invalid")
+ 'description)
+ #f))
(define (check-proper-start description)
(unless (or (properly-starts-sentence? description)
--
2.5.3
Are you OK with the new commit log?
>> Usage of ‘false-if-exception’ made me realize that ‘emit-warning’ is not
>> nicely composable. What about making it return ‘#f’ in order to compose
>> checks and warning together as boolean expressions? Is that idiomatic?
>> Maybe you have a better suggestion?
>
> Not sure I follow. Those ‘check’ procedures are only called for effect,
> not for value, right?
My first idea was to have something similar to this kind of
composability...
--8<---------------cut here---------------start------------->8---
(define (check-texinfo-markup description)
(or (false-if-exception (texi->plain-text description))
(emit-warning package
(_ "Texinfo markup in description is invalid")
'description)))
(define (check-proper-start description)
(or (properly-starts-sentence? description)
(string-prefix-ci? (package-name package) description)
(emit-warning package
(_ "description should start with an upper-case letter or
digit")
'description)))
--8<---------------cut here---------------end--------------->8---
... but with better semantics. IMO this would require more deep thought
so let's forget about this for now. :)
--
Mathieu Lirzin
