guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#41253] [PATCH v3] guix repl: Add script execution.


From: Ludovic Courtès
Subject: [bug#41253] [PATCH v3] guix repl: Add script execution.
Date: Thu, 04 Jun 2020 17:06:16 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux)

Hello!

Konrad Hinsen <konrad.hinsen@fastmail.net> skribis:

> * guix/scripts/repl.scm: Add filename options for script execution.
> * doc/guix.texi (Invoking guix repl): Document it.
> * tests/guix-repl.sh: Test it.

I like it!

It cannot be used as a shebang, can it?

There’s also the Makefile.am change that could be mentioned in the
commit log.

Some comments:

> +When at least one @var{files} argument is provided, @var{files} are
> +executed as Guile scripts in the given order:
> +
> +@example
> +$ guix repl my-script.scm
> +@end example
> +
> +To pass arguments to the script, use @code{--} to prevent them from
> +being interpreted as arguments to @command{guix repl} itself:
> +
> +@example
> +$ guix repl -- my-script.scm --input=foo.txt
> +@end example

I’d remove “$” from the examples.

> +Without any filename argument, a Guile REPL is started:

s/filename/file name/

>  @item -q
>  Inhibit loading of the @file{~/.guile} file.  By default, that
> -configuration file is loaded when spawning a @code{guile} REPL.
> +configuration file is loaded when executing scripts or spawning
> +a @code{guile} REPL.

I think this change is unnecessary (see below).

> +  (display (G_ "Usage: guix repl [OPTIONS...] [-- FILE ARGS...]
> +In the Guix execution environment, run FILE as a Guile script with
> +command-line arguments ARGS. If no FILE is given, start a Guile REPL,\n"))
                               ^                                       ^
Please add a space and change comma to period.

>                  (lambda (arg result)
> -                  (leave (G_ "~A: extraneous argument~%") arg))
> +                  (alist-cons 'script-args
> +                              (cons arg (cdr (assq 'script-args result)))
> +                              result))

I’d change that to just:

  (append `((script . ,arg) (ignore-dot-guile . #t)) result)

and later we can collect all the values associated with 'script.

I think “script” is clearer than “script-args” (and more in-line with
the naming conventions.)

Last but not least: I think -q should always be implied when running a
script.  ~/.guile is really meant for the REPL, nothing else.

> +  ;; Local Variables:
> +  ;; eval: (put 'call-with-connection 'scheme-indent-function 1)
> +  ;; End:
> +  )

Please move the comment to the bottom, outside the parens, as before,
otherwise this paren will feel lonely.  :-)

Thoughts?

Looks like we’re almost done.  Thank you!

Ludo’.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]