From mboxrd@z Thu Jan  1 00:00:00 1970
Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail
From: =?utf-8?Q?Ludovic_Court=C3=A8s?= <ludo@gnu.org>
Newsgroups: gmane.lisp.guile.devel
Subject: Re: [PATCH] add SRFI-119 / language/wisp to Guile? (new patch,
 squashed)
Date: Sat, 10 Jun 2023 18:40:09 +0200
Message-ID: <87legrs23a.fsf@gnu.org>
References: <87h6w2fkz8.fsf@web.de>
 <f31a073e-6b8c-ce48-2917-53a13d76b108@telenet.be>
 <877cwxe4ar.fsf@web.de>
 <2f7d015d-ceb4-ef8f-b4fe-b69e39b723f8@telenet.be>
 <87357ldqaq.fsf@web.de>
 <1a70460e-11fb-9f5d-0d5f-1eb507d5af0d@telenet.be>
 <87ilg4j65e.fsf@web.de> <87edqsj5vt.fsf@web.de>
 <01212259-37dd-5d67-7bbc-101e01d96d01@telenet.be>
 <1a6c8dda-0124-124c-f932-937a11386ced@gmail.com>
 <87fsb5i912.fsf@web.de>
 <c2ae22ea-fd38-f44c-c7d2-315d0b36a933@gmail.com>
 <87ttzc7gwa.fsf@gnu.org>
 <1e0d07bc-dcf8-fe56-7f16-a72e5df0c20d@telenet.be>
 <875ybr2hk9.fsf@gnu.org> <87v8jrdmk5.fsf@web.de>
 <87jzzr7cba.fsf@web.de> <87v8hc8i8v.fsf@web.de>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214";
	logging-data="16535"; mail-complaints-to="usenet@ciao.gmane.io"
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
Cc: "Dr. Arne Babenhauserheide" <arne_bab@web.de>
To: guile-devel@gnu.org
Original-X-From: guile-devel-bounces+guile-devel=m.gmane-mx.org@gnu.org Sat Jun 10 18:40:51 2023
Return-path: <guile-devel-bounces+guile-devel=m.gmane-mx.org@gnu.org>
Envelope-to: guile-devel@m.gmane-mx.org
Original-Received: from lists.gnu.org ([209.51.188.17])
	by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
	(Exim 4.92)
	(envelope-from <guile-devel-bounces+guile-devel=m.gmane-mx.org@gnu.org>)
	id 1q81dx-00047g-7g
	for guile-devel@m.gmane-mx.org; Sat, 10 Jun 2023 18:40:49 +0200
Original-Received: from localhost ([::1] helo=lists1p.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.90_1)
	(envelope-from <guile-devel-bounces@gnu.org>)
	id 1q81db-0004r3-T7; Sat, 10 Jun 2023 12:40:27 -0400
Original-Received: from eggs.gnu.org ([2001:470:142:3::10])
 by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <ludo@gnu.org>) id 1q81dX-0004qm-T7
 for guile-devel@gnu.org; Sat, 10 Jun 2023 12:40:24 -0400
Original-Received: from fencepost.gnu.org ([2001:470:142:3::e])
 by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <ludo@gnu.org>)
 id 1q81dX-0001sg-GX; Sat, 10 Jun 2023 12:40:23 -0400
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org;
 s=fencepost-gnu-org; h=MIME-Version:In-Reply-To:Date:References:Subject:To:
 From; bh=IqPJmWXzs/lyP7wWNUcFHX4mKFsod5t0CGbxQwDPy1E=; b=DgLvAyS/MVe4cby78bE3
 ClHMQG8TDP5BlFahZNRvrmb6PGF7D9E27Ao6e3zLaLzG3C6WtGqkn9OlrIpj1MoUNkBaApcRpjxYp
 +YJF4L+4hoVW/f8qSxk2cqXg0QzZMuywUV3TtcXFu1yLIQVoIS9AqC0HEXH0z8uJypUcqWKpeSzgo
 DKOeEsgMoaANCdJE4r0c4decQcNSsd8HDV0KvCeNi/38VJBTpToSrvjpf/MoJ31FrNavrhvR4SW2w
 qHotfAQRgsWr8ARVY+1EnsO3vOrWUYZXPG8albxZPp5t7NT2gfUHf/VSzBlvSK4+XlmRckcEEmc79
 WbfWENh5lDe+qA==;
Original-Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (helo=ribbon)
 by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
 (Exim 4.90_1) (envelope-from <ludo@gnu.org>)
 id 1q81dL-00008c-IA; Sat, 10 Jun 2023 12:40:23 -0400
X-URL: http://www.fdn.fr/~lcourtes/
X-Revolutionary-Date: Duodi 22 Prairial an 231 de la =?utf-8?Q?R=C3=A9volu?=
 =?utf-8?Q?tion=2C?= jour de la Camomille
X-PGP-Key-ID: 0x090B11993D9AEBB5
X-PGP-Key: http://www.fdn.fr/~lcourtes/ludovic.asc
X-PGP-Fingerprint: 3CE4 6455 8A84 FDC6 9DB4  0CFB 090B 1199 3D9A EBB5
X-OS: x86_64-pc-linux-gnu
In-Reply-To: <87v8hc8i8v.fsf@web.de> (Arne Babenhauserheide's message of "Mon, 
 01 May 2023 11:54:00 +0200")
X-BeenThere: guile-devel@gnu.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "Developers list for Guile,
 the GNU extensibility library" <guile-devel.gnu.org>
List-Unsubscribe: <https://lists.gnu.org/mailman/options/guile-devel>,
 <mailto:guile-devel-request@gnu.org?subject=unsubscribe>
List-Archive: <https://lists.gnu.org/archive/html/guile-devel>
List-Post: <mailto:guile-devel@gnu.org>
List-Help: <mailto:guile-devel-request@gnu.org?subject=help>
List-Subscribe: <https://lists.gnu.org/mailman/listinfo/guile-devel>,
 <mailto:guile-devel-request@gnu.org?subject=subscribe>
Errors-To: guile-devel-bounces+guile-devel=m.gmane-mx.org@gnu.org
Original-Sender: guile-devel-bounces+guile-devel=m.gmane-mx.org@gnu.org
Xref: news.gmane.io gmane.lisp.guile.devel:21858
Archived-At: <http://permalink.gmane.org/gmane.lisp.guile.devel/21858>

Hi!

"Dr. Arne Babenhauserheide" <arne_bab@web.de> skribis:

>>> Ludovic Court=C3=A8s <ludo@gnu.org> writes:

[...]

>> Given the complexities in changing the way languages are handled (the
>> required discussions, as we=E2=80=99ve seen in the not yet resolved disc=
ussion),
>> 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=E2=80=99m confident the code has been battle-tested (I suppose th=
ere
are minimal changes compared to Wisp, right?), so I=E2=80=99ll comment most=
ly on
cosmetic issues.

> From c7a50f632293cf88f241d45d1fd52fa2f58ce772 Mon Sep 17 00:00:00 2001
> From: Arne Babenhauserheide <arne_bab@web.de>
> 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 =E2=80=98Makefile.am=E2=80=99 file=
s.

> +* SRFI-119::                    Wisp: simpler indentation-sensitive sche=
me.

Please capitalize =E2=80=9CScheme=E2=80=9D (in general we like to pay atten=
tion to
typography, spelling, and language in the manual.)

> +@subsection SRFI-119 Wisp: simpler indentation-sensitive scheme.
> +@cindex SRFI-119
> +@cindex wisp

Same: =E2=80=9CScheme=E2=80=9D, =E2=80=9CWisp=E2=80=9D.

> +The languages shipped in Guile include SRFI-119 (wisp), an encoding of

s/(wisp)/, also referred to as @dfn{Wisp} (for ``Whitespace =E2=80=A6'')/

> +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 <maximedevos@telenet.be>

I see you don=E2=80=99t 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/msg00008.html

Let us know what you=E2=80=99d like to do.

> +(define-module (language wisp)
> +   #:export (wisp-scheme-read-chunk wisp-scheme-read-all=20
> +               wisp-scheme-read-file-chunk wisp-scheme-read-file
> +               wisp-scheme-read-string))

Please remove tabs from this file and indent it =E2=80=9Cthe usual way=E2=
=80=9D.  You
can do that by passing it through Emacs and using =E2=80=98M-x indent-regio=
n=E2=80=99,
or possibly using =E2=80=98guix style -f=E2=80=99.

> +; 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 =E2=80=98define-module=E2=80=
=99 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 (=3D -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 =E2=80=9Clines=E2=80=9D, I suggest defi=
ning a proper
record type, like:

  (define-record-type <input-line>
    (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=20
> +       (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-append "REPR-SYNTAX-" wisp-uuid)))
> +(define repr-unsyntax ; #,
> +       (string->symbol (string-append "REPR-UNSYNTAX-" wisp-uuid)))
> +(define repr-quasisyntax ; #`
> +       (string->symbol (string-append "REPR-QUASISYNTAX-" wisp-uuid)))
> +(define repr-unsyntax-splicing ; #,@
> +       (string->symbol (string-append "REPR-UNSYNTAXSPLICING-" wisp-uuid=
)))

Instead of using these UUIDs, I suggest using known unique objects,
=E2=80=9Cunique=E2=80=9D in the sense of =E2=80=98eq?=E2=80=99.

So it can be:

  (define repr-dot (make-symbol "repr-dot"))  ;uninterned symbol

> +(define (wisp-scheme-read-chunk-lines port)
> +         (let loop
> +           ((indent-and-symbols (list)); '((5 "(foobar)" "\"yobble\"")(3=
 "#t"))
> +             (inindent #t)
> +             (inunderscoreindent (equal? #\_ (peek-char port)))
> +             (incomment #f)
> +             (currentindent 0)
> +             (currentsymbols '())
> +             (emptylines 0))

I=E2=80=99d encourage following the usual naming convention, so =E2=80=98in=
-indent?=E2=80=99,
=E2=80=98in-comment?=E2=80=99, etc.

> +                 ((and inunderscoreindent (and (not (equal? #\space next=
-char)) (not (equal? #\newline next-char))))
> +                   (throw 'wisp-syntax-error "initial underscores withou=
t following whitespace at beginning of the line after" (last indent-and-sym=
bols)))

I suggest using exception objects or SRFI-35 error conditions (they=E2=80=
=99re
interchangeable) carrying information about the king of parsing error
and its location.  That way, the user interface, such as the compiler,
can produce helpful error messages (and internationalized, too).

> +(define (wisp-replace-paren-quotation-repr code)
> +         "Replace lists starting with a quotation symbol by
> +         quoted lists."
> +         (match code
> +             (('REPR-QUOTE-e749c73d-c826-47e2-a798-c16c13cb89dd a ...)
> +                (list 'quote (map wisp-replace-paren-quotation-repr a)))

You need to write these clauses like:

  (match code
    (((? wisp-quote?) body ...)
     =E2=80=A6))

where:

  (define (wisp-quote? obj) (eq? obj repr-quote))

On the use of source properties: they=E2=80=99re not ideal (they rely on a =
side
hash table that=E2=80=99s quite memory-hungry) and it would be nice to cons=
ider
also implementing the =E2=80=9Cannotated read=E2=80=9D interface (info "(gu=
ile)
Annotated Scheme Read").  That can come in a subsequent patch, though.

> +++ b/module/language/wisp/spec.scm
> @@ -0,0 +1,85 @@
> +;; Language interface for Wisp in Guile
> +
> +;;; adapted from guile-sweet: https://gitorious.org/nacre/guile-sweet/so=
urce/ae306867e371cb4b56e00bb60a50d9a0b8353109:sweet/common.scm
> +
> +;;; Copyright (C) 2005--2014 by David A. Wheeler and Alan Manuel K. Glor=
ia
> +;;; Copyright (C) 2014--2023 Arne Babenhauserheide.
> +;;; Copyright (C) 2023 Maxime Devos <maximedevos@telenet.be>
> +
> +;;; Permission is hereby granted, free of charge, to any person
> +;;; obtaining a copy of this software and associated documentation
> +;;; files (the "Software"), to deal in the Software without
> +;;; restriction, including without limitation the rights to use, copy,
> +;;; modify, merge, publish, distribute, sublicense, and/or sell copies
> +;;; of the Software, and to permit persons to whom the Software is
> +;;; furnished to do so, subject to the following conditions:
> +;;;
> +;;; The above copyright notice and this permission notice shall be
> +;;; included in all copies or substantial portions of the Software.
> +;;;
> +;;; THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> +;;; EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> +;;; MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> +;;; NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> +;;; BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> +;;; ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> +;;; CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> +;;; SOFTWARE.

Can you make it LGPLv3+?  It=E2=80=99s a small file anyway.

> +(define-language wisp
> +  #:title "Wisp Scheme Syntax. See SRFI-119 for details."
> +  ; . #:reader read-one-wisp-sexp
> +  #:reader read-one-wisp-sexp ; : lambda (port env) : let ((x (read-one-=
wisp-sexp port env))) (display x)(newline) x ;
> +  #:compilers `((tree-il . ,compile-tree-il))
> +  #:decompilers `((tree-il . ,decompile-tree-il))
> +  #:evaluator (lambda (x module) (primitive-eval x))
> +  #:printer write ; TODO: backtransform to wisp? Use source-properties?
> +  #:make-default-environment
> +  (lambda ()
> +    ;; Ideally we'd duplicate the whole module hierarchy so that `set!',
> +    ;; `fluid-set!', etc. don't have any effect in the current environme=
nt.
> +    (let ((m (make-fresh-user-module)))
> +      ;; Provide a separate `current-reader' fluid so that
> +      ;; compile-time changes to `current-reader' are
> +      ;; limited to the current compilation unit.
> +      (module-define! m 'current-reader (make-fluid))
> +      ;; Default to `simple-format', as is the case until
> +      ;; (ice-9 format) is loaded. This allows
> +      ;; compile-time warnings to be emitted when using
> +      ;; unsupported options.
> +      (module-set! m 'format simple-format)

I=E2=80=99d remove this last statement, I think we should avoid fiddling wi=
th
bindings here.

> +(define-module (test-srfi-119)
> +  #:use-module (test-suite lib)
> +  #:use-module (srfi srfi-1)
> +  #:use-module (language wisp))

The test suite looks rather short; it would be nice if it would cover
more code.

In particular, I think source location info is important for front-ends
like this, because it determines the quality of error messages and thus
its ease of use.  Perhaps you could add test in that direction?

Overall I feel we should be able to merge this rather soon.  Do ping me
or Andy when needed!

Thanks,
Ludo=E2=80=99.