diff mbox

[bug#60365,0/4] Rationalize 'PS1' handling in home, system, and shell

Message ID 878rij6xsy.fsf_-_@gnu.org
State New
Headers

Commit Message

Ludovic Courtès Jan. 3, 2023, 10:46 p.m. UTC 
Hi Andrew!

Andrew Tropin <andrew@trop.in> skribis:

>> I found some things could be improved in how we deal with ‘PS1’ in
>> Guix Home, Guix System, and ‘guix shell’ (my initial motivation was
>> making it easy to change ‘PS1’ from a Home configuration).
>>
>> These patches fiddle with ‘PS1’.  The most visible effect is that
>> Guix Home’s ~/.bashrc will no longer provide a default ‘PS1’.
>>
>> Thoughts?
>
> Factorization of bashrc is great!
>
> Not sure about moving PS1 to /etc/bashrc as it won't work on foreign
> distros and guix environment won't be noticable in the prompt.  Warning
> is useful, but I would expect prompt work OOB, when guix-defualts? is
> #t.

Yes, you’re right.

> The possible alternative solution is to factorize bash_profile and put
> PS1 here, so both skeleton and home provided bashrc have it, after that
> setting PS1 with environment-variables field will start working.

How about this:
That way, if one adds ‘PS1’ to ‘environment-variables’, that will
override the default.

WDYT?

Thanks for your feedback!

Ludo’.

Comments

Andrew Tropin Jan. 4, 2023, 6 a.m. UTC | #1 
On 2023-01-03 23:46, Ludovic Courtès wrote:

> Hi Andrew!
>
> Andrew Tropin <andrew@trop.in> skribis:
>
>>> I found some things could be improved in how we deal with ‘PS1’ in
>>> Guix Home, Guix System, and ‘guix shell’ (my initial motivation was
>>> making it easy to change ‘PS1’ from a Home configuration).
>>>
>>> These patches fiddle with ‘PS1’.  The most visible effect is that
>>> Guix Home’s ~/.bashrc will no longer provide a default ‘PS1’.
>>>
>>> Thoughts?
>>
>> Factorization of bashrc is great!
>>
>> Not sure about moving PS1 to /etc/bashrc as it won't work on foreign
>> distros and guix environment won't be noticable in the prompt.  Warning
>> is useful, but I would expect prompt work OOB, when guix-defualts? is
>> #t.
>
> Yes, you’re right.
>
>> The possible alternative solution is to factorize bash_profile and put
>> PS1 here, so both skeleton and home provided bashrc have it, after that
>> setting PS1 with environment-variables field will start working.
>
> How about this:
>
> diff --git a/gnu/home/services/shells.scm b/gnu/home/services/shells.scm
> index d7dd579293..8d7db6b774 100644
> --- a/gnu/home/services/shells.scm
> +++ b/gnu/home/services/shells.scm
> @@ -407,6 +407,9 @@ (define* (file-if-not-empty field #:optional (extra-content #f))
>  # Honor per-interactive-shell startup file
>  if [ -f ~/.bashrc ]; then source ~/.bashrc; fi
>  "
> +        (if (home-bash-configuration-guix-defaults? config)
> +            "export PS1=...\n"

Yep, something like that should work.  Ideally, is to make
%default-bash-profile or even more generic %default-profile and reuse it
across system and home, but it may be not that trivial, so we can
postpone it for now.

> +            "")
>          (serialize-field 'bash-profile)
>          (serialize-field 'environment-variables)))
>  
>
> That way, if one adds ‘PS1’ to ‘environment-variables’, that will
> override the default.
>
> WDYT?
>
> Thanks for your feedback!
>
> Ludo’.
Ludovic Courtès Jan. 5, 2023, 2:20 p.m. UTC | #2 
Andrew Tropin <andrew@trop.in> skribis:

> On 2023-01-03 23:46, Ludovic Courtès wrote:

[...]

>>> Not sure about moving PS1 to /etc/bashrc as it won't work on foreign
>>> distros and guix environment won't be noticable in the prompt.  Warning
>>> is useful, but I would expect prompt work OOB, when guix-defualts? is
>>> #t.
>>
>> Yes, you’re right.
>>
>>> The possible alternative solution is to factorize bash_profile and put
>>> PS1 here, so both skeleton and home provided bashrc have it, after that
>>> setting PS1 with environment-variables field will start working.
>>
>> How about this:
>>
>> diff --git a/gnu/home/services/shells.scm b/gnu/home/services/shells.scm
>> index d7dd579293..8d7db6b774 100644
>> --- a/gnu/home/services/shells.scm
>> +++ b/gnu/home/services/shells.scm
>> @@ -407,6 +407,9 @@ (define* (file-if-not-empty field #:optional (extra-content #f))
>>  # Honor per-interactive-shell startup file
>>  if [ -f ~/.bashrc ]; then source ~/.bashrc; fi
>>  "
>> +        (if (home-bash-configuration-guix-defaults? config)
>> +            "export PS1=...\n"
>
> Yep, something like that should work.  Ideally, is to make
> %default-bash-profile or even more generic %default-profile and reuse it
> across system and home, but it may be not that trivial, so we can
> postpone it for now.

I adjusted things as proposed and pushed:

  b5e3701f1a environment: Avoid false positive on 'PS1' check.
  632e2f7ce1 environment: Simplify 'PS1' suggestion on '--check'.
  086df6ef21 system: Define default 'PS1' in /etc/bashrc rather than ~/.bashrc.
  2127e56bd8 system, home: Factorize default '.bashrc'.

Thanks!

Ludo’.
diff mbox

Patch

diff --git a/gnu/home/services/shells.scm b/gnu/home/services/shells.scm
index d7dd579293..8d7db6b774 100644
--- a/gnu/home/services/shells.scm
+++ b/gnu/home/services/shells.scm
@@ -407,6 +407,9 @@  (define* (file-if-not-empty field #:optional (extra-content #f))
 # Honor per-interactive-shell startup file
 if [ -f ~/.bashrc ]; then source ~/.bashrc; fi
 "
+        (if (home-bash-configuration-guix-defaults? config)
+            "export PS1=...\n"
+            "")
         (serialize-field 'bash-profile)
         (serialize-field 'environment-variables)))