On Sun, Nov 15, 2015 at 9:59 PM, Ludovic Courtès <ludo@gnu.org> wrote:
> Federico Beffa <beffa@ieee.org> skribis:
>> * guix/import/cabal.scm (lex-word): Add support for tests with no spaces.
>>   (impl): Fix handling of operator "==".
>
> LGTM, but I think it’d be great to add a test that illustrates the case
> that this fixes (and to make sure it doesn’t come back later.)

I've rewritten 'impl' and the new test that I've added covers this and more.

>> From f796d814821289a98e401a3e3df13334a2e8689b Mon Sep 17 00:00:00 2001
>> From: Federico Beffa <beffa@fbengineering.ch>
>> Date: Wed, 11 Nov 2015 15:31:46 +0100
>> Subject: [PATCH 3/6] import: hackage: Make it resilient to missing final
>>  newline.
>>
>> * guix/import/cabal.scm (peek-next-line-indent): Check for missing final
>>   newline.
>
> [...]
>
>> +  (if (eof-object? (peek-char port))
>> +      ;; If the file is missing the #\newline on the last line, add it and act
>> +      ;; as if it were there. This is needed for propoer operation of
>                                                       ^^^^
> Typo.
>
>> +      ;; indentation based block recognition.
>> +      (begin (unread-char #\newline port) (read-char port) 0)
>
> Isn’t this equivalent to: 0 ?

No. This is because at the start of a new line we check if and how
many indentation blocks have ended. If the last line doesn't terminate
this check is no done.

>
> Could you add a test for this one?

I've removed the final newline from the test 'test-read-cabal-1".

>
>> From 225164d2355afd6f9455251d87cbd34b08f68cdb Mon Sep 17 00:00:00 2001
>> From: Federico Beffa <beffa@fbengineering.ch>
>> Date: Wed, 11 Nov 2015 16:20:45 +0100
>> Subject: [PATCH 4/6] import: hackage: Make parsing of tests and fields more
>>  flexible.
>>
>> * guix/import/cabal.scm (is-test): Allow spaces between keyword and
>>   parentheses.
>>   (is-id): Add argument 'port'.  Allow spaces between keyword and column.
>>   (lex-word): Adjust call to 'is-id'.
>
> LGTM, and would be perfect with a test.  ;-)

These are now exercised in "test-read-cabal-1".

> [...]
>
>> +(test-equal "canonical-newline-port"
>> +  "This is a journey"
>> +  (let ((port (open-string-input-port
>> +               "This is a journey\r\n")))
>> +    (get-line (canonical-newline-port port))))
>
> I would rather use ‘get-string-all’ and make sure the result is exactly:
>
>   "This is a journey\n"
>
> (Because ‘get-line’ could have been doing its own thing regardless of
> the EOL style.)
>
> A test with several lines, including lines with just \n would be nice.

OK. I've updated it and the test.

>
>> From c57be8cae9b3642beff1462acd32a0aee54ad7c6 Mon Sep 17 00:00:00 2001
>> From: Federico Beffa <beffa@fbengineering.ch>
>> Date: Sat, 14 Nov 2015 15:15:00 +0100
>> Subject: [PATCH 6/6] import: hackage: Handle CRLF end of line style.
>>
>> * guix/import/hackage.scm (hackage-fetch, hackage->guix-package): Do it.
>
> Rather “Use ‘canonical-newline-port’.” instead of “Do it.”

OK.

I've made 1 more change. The importer now peeks at the 'ghc' package
version and uses that as default implementation. Before, without using
the '-e' option, it was assuming "ghc", but no specific version.

Regards,
Fede