Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Refactor nixos/bash prompt using bash function #389305

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

goyalyashpal
Copy link

@goyalyashpal goyalyashpal commented Mar 12, 2025

It simplifies the flow of logic and makes it easier to understand.

i was using this value (though read from a .sh file) in my programs.bash.promptInit for 6 months, and it (seems) to work well with the default gnome terminal emulator and also the Alt-F2 terminals.

however, i have not tested it anywhere else nor tested it in this form (i.e. making changes to the underlying implementation itself)

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Mar 12, 2025
@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Mar 12, 2025
Comment on lines 85 to 96
set_prompt() {
if [ "$TERM" == "dumb" ] || [ ! -n "$INSIDE_EMACS" ]; then
return
fi

local PROMPT_COLOR="1;31m"
((UID)) && PROMPT_COLOR="1;32m"
if [ -n "$INSIDE_EMACS" ]; then
# Emacs term mode doesn't support xterm title escape sequence (\e]0;)
PS1="\n\[\033[$PROMPT_COLOR\][\u@\h:\w]\\$\[\033[0m\] "
else
PS1="\n\[\033[$PROMPT_COLOR\][\[\e]0;\u@\h: \w\a\]\u@\h:\w]\\$\[\033[0m\] "

# Emacs term mode doesn't support xterm title escape sequence (\e]0;)
if [ ! -n "$INSIDE_EMACS" ]; then
local TITLE="\[\e]0;\u@\h: \w\a\]"
fi
Copy link
Author

@goyalyashpal goyalyashpal Mar 12, 2025

Choose a reason for hiding this comment

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

oh wait, forgot inverting the ors to ands...


as the same condition[ ! -n "$INSIDE_EMACS" ] is showing up in both these blocks, and it being true in first block results in return rightaway; so, does it mean that the latter block (setting title) will never be run and so can be eliminated?

i tried git-blaming with github's UI, and sifted through these commits; but couldn't figure out the reason for the existence of second block:

            if [ "$TERM" == "dumb" ] || [ ! -n "$INSIDE_EMACS" ]; then
              return
            fi
            if [ ! -n "$INSIDE_EMACS" ]; then
              local TITLE="\[\e]0;\u@\h: \w\a\]"
            fi

@goyalyashpal goyalyashpal force-pushed the refactor-bash-prompt-invert-select-guard-clauses branch from 48f852a to 87460a5 Compare March 12, 2025 18:50
@@ -87,7 +87,7 @@ in
return
fi
PROMPT_COLOR="1;31m"
local PROMPT_COLOR="1;31m"
Copy link
Author

Choose a reason for hiding this comment

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

this use of local is based on the observation that this bash variable ain't used anywhere outside this prompt configuration (atleast in this file).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants