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

nixos/zsh: do not set ZSH prompt to the 'walters' style #38535

Closed
wants to merge 1 commit into from

Conversation

giraffito
Copy link
Contributor

This is not the default upstream and causes differences in behavior for users compared to other distros.

The commit introducing this default was the one introducing support for zsh, without any justification at the time.

In addition to having surprising effects on user configurations, the prompt is currently buggy and can hide command output: #30121

Motivation for this change

Other distros do not change the default zsh prompt theme, and it changes the behavior of any user configuration that does not explicitly override all settings set in the 'walters' theme. In addition, the current prompt theme is buggy and hides the last line of output from commands that do not terminate their output with a newline.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

This is not the default upstream and causes differences in behavior for users compared to other distros.

The commit introducing this default was the one introducing support for zsh, without any justification at the time.

In addition to having surprising effects on user configurations, the prompt is currently buggy and can hide command output: NixOS#30121
@hedning
Copy link
Contributor

hedning commented Apr 7, 2018

This seems sane, especially since it eats lines, which can be easily encountered by running eg. nix eval nixpkgs.hello.name --raw.

@@ -69,7 +69,7 @@ in

promptInit = mkOption {
default = ''
autoload -U promptinit && promptinit && prompt walters
autoload -U promptinit && promptinit
Copy link
Member

@Mic92 Mic92 Apr 7, 2018

Choose a reason for hiding this comment

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

I know other distros do not that anything, but they also do not have configuration management that let user's override.
The default prompt of zsh is almost useless. Could we at least have something that is consistent with our bash prompt?

          # Provide a nice prompt if the terminal supports it.
          if [ "$TERM" != "dumb" -o -n "$INSIDE_EMACS" ]; then
            PROMPT_COLOR="1;31m"
            let $UID && PROMPT_COLOR="1;32m"
            PS1="\n\[\033[$PROMPT_COLOR\][\u@\h:\w]\\$\[\033[0m\] "
            if test "$TERM" = "xterm"; then
              PS1="\[\033]2;\h:\u:\w\007\]$PS1"
            fi
          fi

it does not have to be fancy pants, but it should be a bit saner to what zsh provide by default.
The reason is, that in automatically configured systems, you don't want to spend time on setting up dotfiles.
Also when users configure their shells the first time they also should not suffer from eye bleeding.

Copy link
Contributor Author

@giraffito giraffito Apr 22, 2018

Choose a reason for hiding this comment

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

I'm not enough of a zsh expert to know how to write an idiomatic configuration, but I would like to avoid having a prompt theme injected by default since this changes the effect of dotfiles atop a regular install compared to other distros.

To me, it seems like automated deploys using zsh will also have custom configuration for it, since zsh is not a default shell in the first place (opinionated sysadmins tend to come with their own dotfiles :P).

Unless someone knows of a way to make the default zsh configuration friendly which will not interfere with other nixos-provided zsh configuration nor user dotfiles, I think we should merge this as-is. Users are used to adding their own config, but shouldn't have to contend with unexpected pre-existing configuration that isn't familiar from other distros or zsh experience.

Copy link
Member

Choose a reason for hiding this comment

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

Anything that runs afterwards, including traditional dotfiles like ~/.zshrc will override the system defaults. Alternatively it can be disabled globally with programs.zsh.promptInit = mkForce "";, the option should probably be types.str instead of types.lines then regular assignment would just work.

Copy link

@pkral78 pkral78 Dec 29, 2019

Choose a reason for hiding this comment

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

Note that disabling zsh on global system level and leave it enabled only at home manager user level, eliminate this weird (IMHO very uncommon) option.

@oxij
Copy link
Member

oxij commented Apr 23, 2018 via email

@oxij
Copy link
Member

oxij commented Apr 23, 2018 via email

@oxij
Copy link
Member

oxij commented Apr 23, 2018 via email

@Mic92
Copy link
Member

Mic92 commented Apr 24, 2018

Is the proper fix for walter just?

setopt prompt_sp

@Mic92
Copy link
Member

Mic92 commented Apr 24, 2018

Has somebody informed zsh upstream about it?

@oxij
Copy link
Member

oxij commented Apr 24, 2018

@Mic92 Interestingly, from my point of view github seems to ignore the messages you write (I don't receive email notifications specifically about your messages).

The fix I will test, but how does it relate to the changes in zsh-users/zsh@43e55a9 (mentioned in #30121)?

Has somebody informed zsh upstream about it?

I did not.

@dtzWill
Copy link
Member

dtzWill commented Sep 10, 2018

Ping? :)

@oxij
Copy link
Member

oxij commented Sep 10, 2018

It's on my TODO list for 18.09.

@mmahut
Copy link
Member

mmahut commented Aug 7, 2019

Any updates on this pull request, please?

@rummik
Copy link
Contributor

rummik commented Aug 13, 2019

I've been switching to home-manager for personal dotfiles management, and this took quite a bit of digging to find where my theme was being mangled. It's very non-obvious that NixOS is setting the prompt to something other than the ZSH default.

oxij added a commit to oxij/nixpkgs that referenced this pull request Aug 19, 2019
See NixOS#38535, properly fixing the prompt seems complicated, and this seems
to work in all the ttys I checked.

Suggested by @Mic92.
@oxij
Copy link
Member

oxij commented Aug 19, 2019 via email

@rummik
Copy link
Contributor

rummik commented Aug 20, 2019

@oxij @matthewbauer I'm still seeing issues even with #66992 applied locally; specifically because the walter theme is setting RPS1
image

@matthewbauer
Copy link
Member

@oxij @matthewbauer I'm still seeing issues even with #66992 applied locally; specifically because the walter theme is setting RPS1
image

Does unset RPS1 help with this?

@rummik
Copy link
Contributor

rummik commented Aug 20, 2019 via email

@matthewbauer
Copy link
Member

Yeah, let's leave this PR open in case we want to just use the default zsh theme.

@matthewbauer matthewbauer reopened this Aug 21, 2019
@oxij
Copy link
Member

oxij commented Aug 23, 2019 via email

@rummik
Copy link
Contributor

rummik commented Aug 23, 2019

@oxij #66992 doesn't resolve this, as the walters theme exhibits unexpected behavior compared to the typical ZSH defaults because it modifies values other than PS1. Regardless of how usable the ZSH default theme is, I would argue that any theme being used as a default should not require a user override RPS1, or any other values, in order to restore expected behavior when setting a theme

Edit: Additionally, I'd also argue that if NixOS is specifying a theme for ZSH, it should either set it for other shells as well, or not set one at all, in order to maintain consistency in this behavior

@oxij
Copy link
Member

oxij commented Aug 23, 2019 via email

@rummik
Copy link
Contributor

rummik commented Aug 24, 2019

@oxij I'm mainly talking about the non-standard prompt changes because that's the scope of the issue. However, if they're creating side effects that require debugging in order to determine where they're being set, much like I was forced to do here, then yes, I think they're better used as example values rather than presets.

@oxij
Copy link
Member

oxij commented Aug 24, 2019 via email

@rummik
Copy link
Contributor

rummik commented Aug 24, 2019

@oxij Yes, one of the first places I looked was /etc/zshrc. I then had to figure out which NixOS option was causing this unexpected behavior to override it

Edit: I just don't think we should be forcing people to override a non-standard theme in order to restore expected behavior. To my knowledge, no NixOS-supplied default theme of other shells set RPS1 -- and if they do, I would argue they should not be doing this either.

Edit: Additionally, I find it strange that anyone should have to argue to restore expected defaults, as opposed to arguing to diverge from expected defaults.

@rummik
Copy link
Contributor

rummik commented Sep 1, 2019

Additionally, it seems like the standard is for NixOS to not be opinionated -- or at least minimally opinionated -- in its default configurations, and this option seems highly opinionated

@stale
Copy link

stale bot commented Jun 26, 2020

Thank you for your contributions.

This has been automatically marked as stale because it has had no activity for 180 days.

If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.

Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse.
  3. Ask on the #nixos channel on irc.freenode.net.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 26, 2020
@rummik
Copy link
Contributor

rummik commented Jul 20, 2020

I'd still like to see this change implemented, since it's very strange and confusing to have this as a default when ZSH users are likely to set their own PS1 using some other means, and this creates major side effects (which is even noted in the current promptInit default).

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 20, 2020
@pinpox
Copy link
Member

pinpox commented Oct 2, 2020

I case anyone else comes across this, having problems with the right side of the promp (RPS1) showing up, even when the prompt is changed to something else (tested with the pure prompt theme):

This workround worked for me:

programs.zsh = {
  enable = true;
  sessionVariables = {
    RPS1 = ""; # Disable the right side prompt that "walters" theme introduces
  };
};

@stale
Copy link

stale bot commented Jun 4, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 4, 2021
@pinpox
Copy link
Member

pinpox commented Jun 5, 2021

Any updates on this @giraffito ?

@rummik
Copy link
Contributor

rummik commented Jun 26, 2021

@pinpox A quick look at their account makes me think they're no longer active on GitHub

I'd be more than happy to submit a new PR with a less opinionated default if the option's available though

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 26, 2021
rummik added a commit to rummik/nixpkgs that referenced this pull request Jun 26, 2021
This resolves a long-standing issue caused by the 'walters' theme setting `RPS1`.  See NixOS#38535 for discussion details.
@pinpox
Copy link
Member

pinpox commented Jun 26, 2021

Thanks @rummik ! I'll subscribe to your new PR.

@dasJ
Copy link
Member

dasJ commented Sep 9, 2021

Closing this since the wanted functionality was implemented in #128183

@dasJ dasJ closed this Sep 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet