GNU bug report logs - #68935
[PATCH 0/3] Add 'put' option to guix import.

Previous Next

Package: guix-patches;

Reported by: Herman Rimm <herman <at> rimm.ee>

Date: Mon, 5 Feb 2024 14:52:02 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <at> gnu.org>

Bug is archived. No further changes may be made.

Full log


Message #53 received at 68935 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Herman Rimm <herman <at> rimm.ee>
Cc: Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Ricardo Wurmus <rekado <at> elephly.net>,
 68935 <at> debbugs.gnu.org, Christopher Baines <guix <at> cbaines.net>
Subject: Re: [bug#68935] [PATCH v2 4/6] utils: Add find-expression procedure.
Date: Mon, 19 Feb 2024 22:38:32 +0100
Herman Rimm <herman <at> rimm.ee> skribis:

> * guix/utils.scm (find-expression): Add and export procedure.
> * tests/utils.scm ("find-expression"): Add test.
>
> Change-Id: Ie209df39c1f006b20aa6436fb1aef4c84b1694ee

[...]

> +(define (find-expression file expr proc)
> +  "Search in FILE for a top-level expression which alphabetically
> +succeeds EXPR. Call PROC with the location if found, or with #f
> +otherwise."
> +  (let* ((name (match expr
> +                 (('define-public symbol _ ...)
> +                  (symbol->string symbol))))
> +         (source-properties
> +           (call-with-input-file
> +             file
> +             (lambda (port)
> +               (do ((syntax (read-syntax port)
> +                            (read-syntax port)))
> +                 ((match (syntax->datum syntax)
> +                    (('define-public symbol _ ...)
> +                     (string> (symbol->string symbol)
> +                              name))
> +                    ((? eof-object?) #t)
> +                    (_ #f))
> +                  (if (eof-object? syntax)
> +                    #f (syntax-source syntax))))))))
> +    (proc source-properties)))

I think it’d be clearer to:

  1. Omit ‘proc’ and always return the source properties of the thing
     that has been found.

  2. Pass a symbol instead of ‘expr’.

  3. Call it ‘find-definition-insertion-location’ to clarify that it’s
     really about finding where we want to insert a definition, hence
     alphabetical sorting.

BTW, the formatting above is unusual; for instance, ‘file’ would
normally appear on the same line as ‘call-with-input-file’.

Also, write:

  (and (not (eof-object? syntax))
       (syntax-source-syntax))

… instead of using ‘if’.

Thanks,
Ludo’.




This bug report was last modified 1 year and 86 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.