GNU bug report logs - #24450
pypi importer outputs strange character series in optional dependency case.

Previous Next

Full log

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

From: Ricardo Wurmus <ricardo.wurmus <at> mdc-berlin.de>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 24450 <at> debbugs.gnu.org
Subject: Re: [PATCHv2] Re: pypi importer outputs strange character series in
 optional dependency case.
Date: Mon, 10 Jun 2019 11:23:10 +0200

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

>>> +          ;; (extra) requirements.  Non-optional requirements must appear
>>> +          ;; before any section is defined.
>>> +          (if (or (eof-object? line) (section-header? line))
>>> +              (reverse result)
>>> +              (cond
>>> +               ((or (string-null? line) (comment? line))
>>> +                (loop result))
>>> +               (else
>>> +                (loop (cons (clean-requirement line)
>>> +                            result))))))))))
>>> +
>>
>> I think it would be better to use “match” here instead of nested “let”,
>> “if” and “cond”.  At least you can drop the “if” and just use cond.
>>
>> The loop let and the inner let can be merged.
>
> I'm not sure I understand; wouldn't merging the named let with the plain
> let mean adding an extra LINE argument to my LOOP procedure?  I don't
> want that.

Let’s forget about merging the nested “let”, because you would indeed
need to change a few more things.  It’s fine to keep that as it is.  But
(if … (cond …)) is not pretty.  At least it could be done in one “cond”:

    (cond
     ((or (eof-object? line) (section-header? line))
      (reverse result))
     ((or (string-null? line) (comment? line))
      (loop result))
     (else
      (loop (cons (clean-requirement line)
                  result))))

> Also, how could the above code be expressed using "match"? I'm using
> predicates which tests for (special) characters in a string; I don't see
> how the more primitive pattern language of "match" will enable me to do
> the same.

“match” has support for predicates, so you could do something like this:

    (match line
     ((or (eof-object) (? section-header?))
      (reverse result))
     ((or '() (? comment?))
      (loop result))
     (_ (loop (cons (clean-requirement line) result))))

This allows you to match “eof-object” and '() directly.  Whenever I see
“string-null?” I think it might be better to “match” on the empty list
directly.

But really, that’s up to you.  I only feel strongly about avoiding “(if
… (cond …))”.

--
Ricardo

Previous Next

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