Mark H Weaver <m...@netris.org> skribis:
> From 951b9d224d84bfec271b51615bc095013d153694 Mon Sep 17 00:00:00 2001
> From: Mark H Weaver <m...@netris.org>
> Date: Sat, 6 Apr 2013 23:19:55 -0400
> Subject: [PATCH 3/3] Add keyword arguments to file opening procedures.
>
> * libguile/fports.c (scm_open_file_with_encoding): New API function,
> containing the code previously found in 'scm_open_file', but modified
> to accept the new 'guess_encoding' and 'encoding' arguments.
>
> (scm_open_file): Now just a simple wrapper that calls
> 'scm_open_file_with_encoding'.
>
> (scm_i_open_file): New implementation of 'open-file' that accepts
> keyword arguments '#:guess-encoding' and '#:encoding', and calls
> 'scm_open_file_with_encoding'.
>
> (scm_init_fports_keywords): New initialization function that gets
> called after keywords are initialized.
>
> * libguile/fports.h (scm_open_file_with_encoding,
> scm_init_fports_keywords): Add prototypes.
>
> * libguile/init.c (scm_i_init_guile): Call 'scm_init_fports_keywords'.
>
> * module/ice-9/boot-9.scm: Add enhanced versions of 'open-input-file',
> 'open-output-file', 'call-with-input-file', 'call-with-output-file',
> 'with-input-from-file', 'with-output-to-file', and
> 'with-error-to-file', that accept keyword arguments '#:binary',
> '#:encoding', and (for input port constructors) '#:guess-encoding'.
>
> * doc/ref/api-io.texi (File Ports): Update documentation.
>
> * test-suite/tests/ports.test ("keyword arguments for file openers"):
> Add tests.
Looks good.
Minor comments:
> +@deffn {Scheme Procedure} open-file filename mode @
> + [#:guess-encoding=#f] [#:encoding=#f]
> +@deffnx {C Function} scm_open_file_with_encoding @
> + (filename, mode, guess_encoding, encoding)
> @deffnx {C Function} scm_open_file (filename, mode)
> Open the file whose name is @var{filename}, and return a port
> representing that file. The attributes of the port are
> @@ -900,8 +903,17 @@ to the underlying @code{open} call. Still, the flag is
> generally useful
> because of its port encoding ramifications.
> @end table
>
> -If a file cannot be opened with the access
> -requested, @code{open-file} throws an exception.
> +Unless binary mode is requested, the character encoding of the new port
> +is determined as follows: First, if @var{guess-encoding} is true,
> +heuristics will be used to guess the encoding of the file. If it is
“heuristics” is vague. I’d prefer “the ‘file-encoding’ procedure is
called to check for Emacs-style coding declarations (@pxref{Character
Encoding of Source Files})”. Should BOMs also be mentioned?
> +/* scm_open_file_with_encoding
> * Return a new port open on a given file.
> *
> + * Use heuristics to guess the encoding is GUESS_ENCODING
> + * is true, else use ENCODING if not false, else use the
> + * default port encoding.
Likewise.
And you’re welcome to remove the leading stars also. :-)
> +SCM k_guess_encoding = SCM_UNDEFINED;
> +SCM k_encoding = SCM_UNDEFINED;
Add ‘static’.
> + scm_c_bind_keyword_arguments (FUNC_NAME, keyword_args, 0,
> + k_guess_encoding, &guess_encoding,
> + k_encoding, &encoding,
> + SCM_UNDEFINED);
Comes in handy. ;-)
> +(define* (open-input-file
> + str #:key (binary #f) (encoding #f) (guess-encoding #f))
> + "Takes a string naming an existing file and returns an input port
> +capable of delivering characters from the file. If the file
> +cannot be opened, an error is signalled."
It’s a detail, for these procedures, I would s/str/file/, and in
docstrings s/file STR/FILE/.
The test suite looks comprehensive, that’s great.
Thanks!
Ludo’.
