Re: [PATCH] environment: Add a prompt-name argument.

[Roel Janssen]

Ludovic Courtès writes:

> Hi,
>
> Roel Janssen <address@hidden> skribis:
>
>> This patch adds a -p and --prompt-name argument to @code{guix
>> environment}, and puts the argument's value between the square brackets
>> instead of "env".
>
> Sounds like a good idea!  Maybe ‘--prompt-suffix’ would be more accurate
> a name?  WDYT?

I think the goal is to identify the environments by name, and whether
that's by adding a suffix or a prefix doesn't really matter.  So I
believe we should emphasize on "name" and not on "suffix".

I wanted to use --name first, but it doesn't explain why we would use -p
for the short option instead of -n (which is already taken by --dry-run).

WDYT?

>>>From ea958e847019c94a2bde49285f1436dfec72e570 Mon Sep 17 00:00:00 2001
>> From: Roel Janssen <address@hidden>
>> Date: Thu, 20 Oct 2016 13:07:15 +0200
>> Subject: [PATCH] environment: Add a prompt-name argument.
>>
>> * guix/scripts/environment.scm: Add --prompt-name (-p) argument.
>
> Some cosmetic suggestions follow…  :-)
>
>>  (define* (launch-environment/container #:key command bash user-mappings
>> -                                       profile paths network?)
>> +                                       profile paths network? prompt-name)
>
> Make it
>
>   (prompt-suffix %default-prompt-suffix)
>
> with:
>
>   (define %default-prompt-suffix
>     ;; The default prompt suffix
>     " [env]")
>
> such that #:prompt-suffix is always a string.
>
>> +            (setenv "PS1" (string-append "address@hidden \\w ["
>> +                                         (if (not prompt-name)
>> +                                             "env"
>> +                                             prompt-name)
>
> Assume ‘prompt-name’ is a string.
>
>> +    (let* ((opts        (parse-args args))
>> +           (pure?       (assoc-ref opts 'pure))
>> +           (container?  (assoc-ref opts 'container?))
>> +           (network?    (assoc-ref opts 'network?))
>> +           (prompt-name (assoc-ref opts 'prompt-name))
>
> I’d change ‘%default-options’ like this:
>
>   (define %default-options
>     `(…
>       (prompt-suffix . ,%default-prompt-suffix)))
>
>> +                                                  #:prompt-name prompt-name
>
> Maybe add the brackets here?  Like:
>
>   #:prompt-suffix (match prompt-suffix
>                     ("" "")  ;allow for an empty suffix
>                     ((? string? suffix) (string-append " [" suffix "]")))
>
> Thanks!
>
> Ludo’.

These are great suggestions!  I will adapt it when we agree
upon the long option name :).

Thanks!

Kind regards,
Roel Janssen