Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

bash: fix PS1=abc bash #23161

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

john-peterson
Copy link
Contributor

if the shell already has a name don't change it. this is the same behaviour as bash itself

env -u PS1 bash --noprofile --norc
echo $PS1
\s-\v$

PS1=abc bash --noprofile --norc
echo $PS1
abc

if the shell already has a name don't change it. this is the same behaviour as bash itself

env -u PS1 bash --noprofile --norc
echo $PS1
\s-\v\$

PS1=abc bash --noprofile --norc
echo $PS1
abc
Copy link
Member

@TomJo2000 TomJo2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure what perceived bug you are attempting to fix in this PR.

PS1 is not the name of the shell.
It's the shell prompt.

\s-\v$ is the expected default if no other value is set in the profile ($PREFIX/etc/profile), system bashrc file ($PREFIX/etc/bash.bashrc) or user .bashrc file (~/.bashrc).

The relevant section from man 1 bash1 reads:

PS1    The value of this parameter is expanded (see PROMPTING below)
       and used as the primary prompt string.  The default value is
       ``\s-\v\$ ''.

This default \s-\v\$ is evaluates to2: bash-5.2$
While our default assignment of \[\e[0;32m\]\w\[\e[0m\] \[\e[0;97m\]\$\[\e[0m\] evaluates to2 ~ $ ,
with the path in green (and truncated to 2 segments by the PROMPT_DIRTRIM=2 above), and the $ in white.

It is probably worthwhile to allow users to set a PS1 by passing it along to a spawned bash shell,
though this isn't a usecase I have ever encountered before.

Footnotes

  1. https://man.archlinux.org/man/bash.1#PS1

  2. https://man.archlinux.org/man/bash.1#PROMPTING 2

@@ -12,7 +12,9 @@ export HISTCONTROL=ignoreboth

# Default command line prompt.
PROMPT_DIRTRIM=2
if test -z "$(env | grep -w PS1)"; then
PS1='\[\e[0;32m\]\w\[\e[0m\] \[\e[0;97m\]\$\[\e[0m\] '
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using [[ -v PS1 ]]1 instead to avoid the dependency on grep.

Suggested change
PS1='\[\e[0;32m\]\w\[\e[0m\] \[\e[0;97m\]\$\[\e[0m\] '
# Allow users to override $PS1 by passing it to the invocation of bash as an environment variable
[[ -v PS1 ]] || PS1='\[\e[0;32m\]\w\[\e[0m\] \[\e[0;97m\]\$\[\e[0m\] '

Footnotes

  1. https://man.archlinux.org/man/bash.1#CONDITIONAL_EXPRESSIONS:~:text=set%20builtin%20below.-,%2Dv%20varname,-True%20if%20the

@TomJo2000
Copy link
Member

TERMUX_PKG_HOMEPAGE=https://www.gnu.org/software/bash/
TERMUX_PKG_DESCRIPTION="A sh-compatible shell that incorporates useful features from the Korn shell (ksh) and C shell (csh)"
TERMUX_PKG_LICENSE="GPL-3.0"
TERMUX_PKG_MAINTAINER="Joshua Kahn @TomJo2000"
_MAIN_VERSION=5.2
_PATCH_VERSION=37
TERMUX_PKG_VERSION=${_MAIN_VERSION}.${_PATCH_VERSION}

By the way you'll need to add TERMUX_PKG_REVISION=1 to the bash build.sh.
Preferably under the package version.

Since this is an update to part of the package it should be recognized by the package manager as such.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants