Re: [PATCH 1/2] lint: Check non-translated package descriptions.

[Mathieu Lirzin]

address@hidden (Ludovic Courtès) writes:

> Mathieu Lirzin <address@hidden> 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.

0001-lint-Improve-check-texinfo-markup.patch
Description: Text Data

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