Hi!
"Dr. Arne Babenhauserheide" skribis:
>>> Ludovic Courtès writes:
[...]
>> Given the complexities in changing the way languages are handled (the
>> required discussions, as we’ve seen in the not yet resolved discussion),
>> would you be OK with keeping the question about adding support for
>> SRFI-119 to Guile separate from the general discussion about language
>> handling?
>>
>> Are there improvements needed besides the ones I did thanks to the
>> review by Maxime or is this good to go?
>
> I created a new (squashed) version of the patch to simplify reviewing:
It does, thank you!
Overall I’m confident the code has been battle-tested (I suppose there
are minimal changes compared to Wisp, right?), so I’ll comment mostly on
cosmetic issues.
> From c7a50f632293cf88f241d45d1fd52fa2f58ce772 Mon Sep 17 00:00:00 2001
> From: Arne Babenhauserheide
> Date: Fri, 3 Feb 2023 22:20:04 +0100
> Subject: [PATCH] Add language/wisp, wisp tests, and srfi-119 documentation
>
> * doc/ref/srfi-modules.texi (srfi-119): add node
> * module/language/wisp.scm: New file.
> * module/language/wisp/spec.scm: New file.
> * test-suite/tests/srfi-119.test: New file.
Please add the new files to the relevant ‘Makefile.am’ files.
> +* SRFI-119::Wisp: simpler indentation-sensitive scheme.
Please capitalize “Scheme” (in general we like to pay attention to
typography, spelling, and language in the manual.)
> +@subsection SRFI-119 Wisp: simpler indentation-sensitive scheme.
> +@cindex SRFI-119
> +@cindex wisp
Same: “Scheme”, “Wisp”.
> +The languages shipped in Guile include SRFI-119 (wisp), an encoding of
s/(wisp)/, also referred to as @dfn{Wisp} (for ``Whitespace …'')/
> +Scheme that allows replacing parentheses with equivalent indentation and
> +inline colons. See
Two spaces after end-of-sentence periods, to facilitate navigation in
Emacs.
> +++ b/module/language/wisp.scm
> @@ -0,0 +1,761 @@
> +;;; Wisp
> +
> +;; Copyright (C) 2013, 2017, 2018, 2020 Free Software Foundation, Inc.
> +;; Copyright (C) 2014--2023 Arne Babenhauserheide.
> +;; Copyright (C) 2023 Maxime Devos
I see you don’t have a copyright assignment on file for Guile. If you
wish, you can assign copyright to the FSF:
https://lists.gnu.org/archive/html/guile-devel/2022-10/msg8.html
Let us know what you’d like to do.
> +(define-module (language wisp)
> + #:export (wisp-scheme-read-chunk wisp-scheme-read-all
> + wisp-scheme-read-file-chunk wisp-scheme-read-file
> + wisp-scheme-read-string))
Please remove tabs from this file and indent it “the usual way”. You
can do that by passing it through Emacs and using ‘M-x indent-region’,
or possibly using ‘guix style -f’.
> +; use curly-infix by default
Use two leading colons for comments, except for margin comments.
> +(read-enable 'curly-infix)
This needs to be:
(eval-when (expand load eval)
(read-enable 'curly-infix))
> +(use-modules
> + (srfi srfi-1)
> + (srfi srfi-11); for let-values
> + (ice-9 rw); for write-string/partial
> + (ice-9 match))
Please make them #:use-module clauses in the ‘define-module’ form above.
> +;; Helper functions for the indent-and-symbols data structure: '((indent
> token token ...) ...)
> +(define (line-indent line)
> + (car line))
> +
> +(define (line-real-indent line)
> + "Get the indentation without the comment-marker for unindented
> lines (-1 is treated as 0)."
> + (let (( indent (line-indent line)))
> + (if (= -1 indent)
> + 0
> + indent)))
> +
> +(define (line-code line)
> + (let ((code (cdr line)))
> + ; propagate source properties
> + (when (not (null? code))
> +(set-source-properties! code (source-properties line)))
> + code))
Instead of using lists or pairs for “lines”, I suggest defining a proper
record type, like:
(define-record-type
(input-line indent content)
input-line?
(indent input-line-indent)
(content input-line-content))
This will make the code easier to read and type-clear.
> +; literal values I need
> +(define readcolon
> + (string->symbol ":"))
> +
> +(define wisp-uuid "e749c73d-c826-47e2-a798-c16c13cb89dd")
> +; define an intermediate dot replacement with UUID to avoid clashes.
> +(define repr-dot ; .
> + (string->symbol (string-append "REPR-DOT-" wisp-uuid)))
> +
> +; allow using reader additions as the first element on a line to prefix the
> list
> +(define repr-quote ; '
> + (string->symbol (string-append "REPR-QUOTE-" wisp-uuid)))
> +(define repr-unquote ; ,
> + (string->symbol (string-append "REPR-UNQUOTE-" wisp-uuid)))
> +(define repr-quasiquote ; `
> + (string->symbol (string-append "REPR-QUASIQUOTE-" wisp-uuid)))
> +(define repr-unquote-splicing ; ,@
> + (string->symbol (string-append "REPR-UNQUOTESPLICING-" wisp-uuid)))
> +
> +(define repr-syntax ; #'
> + (string->symbol (string-a