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?= 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> <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> <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" 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: 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 ) 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 ) 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 ) 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 ) 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 ) 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" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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: Hi! "Dr. Arne Babenhauserheide" skribis: >>> Ludovic Court=C3=A8s 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 > 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 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 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 > + > +;;; 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.