Subject: Re: [bug#68935] [PATCH v2 4/6] utils: Add find-expression procedure.

From: Ludovic Courtès <[email-masked]>
Date: Mon, 19 Feb 2024 22:38:32 +0100

Herman Rimm <[email-masked]> 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’.