Federico Beffa <beffa@ieee.org> skribis:
Toggle quote (3 lines)
> On Fri, Nov 13, 2015 at 10:19 PM, Ludovic Courtès <ludo@gnu.org> wrote:
>> Federico Beffa <beffa@ieee.org> skribis:
Toggle quote (7 lines)
>> In practice this discards LF even if it’s not following CR; that’s
>> probably a good enough approximation, but an XXX comment would be
>> welcome.
>
> This is intentional because, in my ignorance, I only know of uses of
> '\r' before or after '\n'. Do you know of any other use in text files?
ISTR that some OSes (MacOS 9 and earlier?! who cares?! :-)) use(d) a
single LF instead of a single CR.
Again that’s fine in practice I guess, but I always think it’s good to
add a note when we make an approximation so we can notice later, just in
case.
Toggle quote (16 lines)
> The attached patches fix the parsing of all but two of the failures
> reported by Paul.
> Two cabal files are still not imported correctly because they are buggy:
>
> * streaming-commons: indentation changes from 4 to 2. But this is
> explicitly forbidden. From [1]: "Field names may be indented, but all
> field values in the same section must use the same indentation."
>
> * fgl: uses braces to delimit the value of a field. As far as I
> understand this is not allowed by [1]: "To continue a field value,
> indent the next line relative to the field name." and "Flags,
> conditionals, library and executable sections use layout to indicate
> structure. ... As an alternative to using layout you can also use
> explicit braces {}. ". Thus I understand that braces may be used to
> delimit sections, not field values.
Toggle quote (3 lines)
> Obviously the official 'cabal' program is more permissive than the
> description in the documentation.
We’re more royalist than the king! ;-)
Toggle quote (11 lines)
> From d13f06383d07e0ad4096ff7eb715264463738b0c Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Wed, 11 Nov 2015 10:39:38 +0100
> Subject: [PATCH 1/6] import: hackage: Add recognition of 'true' and 'false'
> symbols.
>
> * guix/import/cabal.scm (is-true, is-false, lex-true, lex-false): New procedures.
> (lex-word): Use them.
> (make-cabal-parser): Add TRUE and FALSE tokens.
> (eval): Add entries for 'true and 'false symbols.
Toggle quote (8 lines)
> From 445f1b6197c0e266027ac033c52629d990137171 Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Wed, 11 Nov 2015 11:22:42 +0100
> Subject: [PATCH 2/6] import: hackage: Imporve parsing of tests.
>
> * 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.)
Toggle quote (9 lines)
> 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.
Toggle quote (3 lines)
> + (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
Toggle quote (3 lines)
> + ;; indentation based block recognition.
> + (begin (unread-char #\newline port) (read-char port) 0)
Isn’t this equivalent to: 0 ?
Could you add a test for this one?
Toggle quote (11 lines)
> 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. ;-)
Toggle quote (8 lines)
> From 1b26410f4a7a920382750bffbf5381394acafdbc Mon Sep 17 00:00:00 2001
> From: Federico Beffa <beffa@fbengineering.ch>
> Date: Sat, 14 Nov 2015 15:00:36 +0100
> Subject: [PATCH 5/6] utils: Add 'canonical-newline-port'.
>
> * guix/utils.scm (canonical-newline-port): New procedure.
> * tests/utils.scm ("canonical-newline-port"): New test.
Toggle quote (6 lines)
> +(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.
Toggle quote (7 lines)
> 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.”
Thanks for all the work!
Ludo’.
