diff mbox

environment: Add a prompt-name argument.

Message ID 87lgxjp98m.fsf@gnu.org
State New
Headers show

Commit Message

Roel Janssen Oct. 20, 2016, 11:09 a.m. UTC
Dear Guix,

Having multiple virtual terminals open, each running in their own
environment created with @code{guix environment} can become confusing to
the user.  Therefore, I would like to add an option to pass a name to
the shell prompt.  Currently we default to "[env]".

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".

Examples:
[roel@hostname guix]$ guix environment --container --prompt-name="guile" guile
roel@hostname ~/sources/guix [guile]#

[roel@hostname guix]$ guix environment --container -p "guile" guile                                                   
roel@hostname ~/sources/guix [guile]#

[roel@hostname guix]$ guix environment --container guile
roel@hostname ~/sources/guix [env]#

Now, I don't know what the correct naming for the argument is
(prompt-name).  So, I have two questions:

0. Do you think we should apply this patch?
1. What name should the argument have?

Kind regards,
Roel Janssen

>From ea958e847019c94a2bde49285f1436dfec72e570 Mon Sep 17 00:00:00 2001
From: Roel Janssen <roel@gnu.org>
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.
---
 guix/scripts/environment.scm | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

Comments

Ludovic Courtès Oct. 24, 2016, 8:40 p.m. UTC | #1
Hi,

Roel Janssen <roel@gnu.org> 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?

>>From ea958e847019c94a2bde49285f1436dfec72e570 Mon Sep 17 00:00:00 2001
> From: Roel Janssen <roel@gnu.org>
> 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 "\\u@\\h \\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’.
Roel Janssen Oct. 27, 2016, 8:51 a.m. UTC | #2
Ludovic Courtès writes:

> Hi,
>
> Roel Janssen <roel@gnu.org> 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 <roel@gnu.org>
>> 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 "\\u@\\h \\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
Ludovic Courtès Oct. 27, 2016, 12:47 p.m. UTC | #3
Roel Janssen <roel@gnu.org> skribis:

> Ludovic Courtès writes:
>
>> Hi,
>>
>> Roel Janssen <roel@gnu.org> 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".

Makes sense, that also works for me!

In the code I think it’s still more appropriate to use “suffix” when
we’re talking about the suffix, for clarity.

Thanks,
Ludo’.
diff mbox

Patch

diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index 0c69bfc..e36c97e 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -149,6 +149,8 @@  COMMAND or an interactive shell in that environment.\n"))
       --ad-hoc           include all specified packages in the environment instead
                          of only their inputs"))
   (display (_ "
+  -p, --prompt-name      use PROMPT-NAME in the command prompt of the environment"))
+  (display (_ "
       --pure             unset existing environment variables"))
   (display (_ "
       --search-paths     display needed environment variable definitions"))
@@ -237,6 +239,9 @@  COMMAND or an interactive shell in that environment.\n"))
          (option '(#\N "network") #f #f
                  (lambda (opt name arg result)
                    (alist-cons 'network? #t result)))
+         (option '(#\p "prompt-name") #t #f
+                 (lambda (opt name arg result)
+                   (alist-cons 'prompt-name arg result)))
          (option '("share") #t #f
                  (lambda (opt name arg result)
                    (alist-cons 'file-system-mapping
@@ -376,7 +381,7 @@  environment variables are cleared before setting the new ones."
            ((_ . status) status)))))
 
 (define* (launch-environment/container #:key command bash user-mappings
-                                       profile paths network?)
+                                       profile paths network? prompt-name)
   "Run COMMAND within a container that features the software in PROFILE.
 Environment variables are set according to PATHS, a list of native search
 paths.  The global shell is BASH, a file name for a GNU Bash binary in the
@@ -434,7 +439,11 @@  host file systems to mount inside the container."
             (symlink bash "/bin/sh")
 
             ;; Set a reasonable default PS1.
-            (setenv "PS1" "\\u@\\h \\w [env]\\$ ")
+            (setenv "PS1" (string-append "\\u@\\h \\w ["
+                                         (if (not prompt-name)
+                                             "env"
+                                             prompt-name)
+                                         "]\\$ "))
 
             ;; Setup directory for temporary files.
             (mkdir-p "/tmp")
@@ -525,13 +534,14 @@  message if any test fails."
 ;; Entry point.
 (define (guix-environment . args)
   (with-error-handling
-    (let* ((opts       (parse-args args))
-           (pure?      (assoc-ref opts 'pure))
-           (container? (assoc-ref opts 'container?))
-           (network?   (assoc-ref opts 'network?))
-           (bootstrap? (assoc-ref opts 'bootstrap?))
-           (system     (assoc-ref opts 'system))
-           (command    (or (assoc-ref opts 'exec)
+    (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))
+           (bootstrap?  (assoc-ref opts 'bootstrap?))
+           (system      (assoc-ref opts 'system))
+           (command     (or (assoc-ref opts 'exec)
                            ;; Spawn a shell if the user didn't specify
                            ;; anything in particular.
                            (if container?
@@ -604,6 +614,7 @@  message if any test fails."
                                                   #:user-mappings mappings
                                                   #:profile profile
                                                   #:paths paths
+                                                  #:prompt-name prompt-name
                                                   #:network? network?)))
                  (else
                   (return