From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maxim Cournoyer Subject: bug#24450: [PATCHv2] Re: pypi importer outputs strange character series in optional dependency case. Date: Sun, 16 Jun 2019 23:11:01 +0900 Message-ID: <87sgs91uyy.fsf@gmail.com> References: <87pnod7ot4.fsf@gmail.com> <87muiq5d7c.fsf@gmail.com> <87blz5dcap.fsf@mdc-berlin.de> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:470:142:3::10]:34217) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1hcW2s-0008Lj-Qr for bug-guix@gnu.org; Sun, 16 Jun 2019 10:22:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hcVt4-0005I8-F3 for bug-guix@gnu.org; Sun, 16 Jun 2019 10:12:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:55897) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hcVt4-0005Hy-BN for bug-guix@gnu.org; Sun, 16 Jun 2019 10:12:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hcVt4-0008Q3-3U for bug-guix@gnu.org; Sun, 16 Jun 2019 10:12:02 -0400 Sender: "Debbugs-submit" Resent-Message-ID: In-Reply-To: <87blz5dcap.fsf@mdc-berlin.de> (Ricardo Wurmus's message of "Mon, 10 Jun 2019 11:23:10 +0200") List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: "bug-Guix" To: Ricardo Wurmus Cc: 24450@debbugs.gnu.org Hello! Continued feedback about your much appreciated comments! :-) Ricardo Wurmus writes: > Maxim Cournoyer writes: > >>>> + ;; (extra) requirements. Non-optional requirements must ap= pear >>>> + ;; 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 =E2=80=9Cmatch=E2=80=9D here instead = of nested =E2=80=9Clet=E2=80=9D, >>> =E2=80=9Cif=E2=80=9D and =E2=80=9Ccond=E2=80=9D. At least you can drop= the =E2=80=9Cif=E2=80=9D 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=E2=80=99s forget about merging the nested =E2=80=9Clet=E2=80=9D, beca= use you would indeed > need to change a few more things. It=E2=80=99s fine to keep that as it i= s. But > (if =E2=80=A6 (cond =E2=80=A6)) is not pretty. At least it could be done= in one =E2=80=9Ccond=E2=80=9D: > > (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)))) Agreed and fixed, thanks. >> 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. > > =E2=80=9Cmatch=E2=80=9D has support for predicates, so you could do somet= hing like this: > > (match line > ((or (eof-object) (? section-header?)) > (reverse result)) > ((or '() (? comment?)) > (loop result)) > (_ (loop (cons (clean-requirement line) result)))) Oh, that's neat! I had no idea that predicates could be used with "match". '() would need to be replaced by "" to match the empty string. Another gotcha with "match", is that the "or" seems to evaluate every component, no matter if a early true condition was found; this resulted in the following error: --8<---------------cut here---------------start------------->8--- + (wrong-type-arg + "string-trim" + "Wrong type argument in position ~A (expecting ~A): ~S" + (1 "string" #) + (#)) result: FAIL --8<---------------cut here---------------end--------------->8--- Due to the "(or (eof-object) (? section-header?)" match clause evaluating the section-header? predicate despite the line being an EOF character. > This allows you to match =E2=80=9Ceof-object=E2=80=9D and '() directly. = Whenever I see > =E2=80=9Cstring-null?=E2=80=9D I think it might be better to =E2=80=9Cmat= ch=E2=80=9D on the empty list > directly. string-null? and an empty list are not the same, unless I'm missing somethi= ng. > But really, that=E2=80=99s up to you. I only feel strongly about avoidin= g =E2=80=9C(if > =E2=80=A6 (cond =E2=80=A6))=E2=80=9D. Due to the problem mentioned above, I stayed with "cond". Thanks! Maxim