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

Sample prompts #7897

Merged
merged 9 commits into from Apr 8, 2021
Merged

Sample prompts #7897

merged 9 commits into from Apr 8, 2021

Conversation

faho
Copy link
Member

@faho faho commented Apr 3, 2021

Description

This is basically a first pass on the sample prompts.

It removes:

  • classic and classic+status - they are just subsets of classic+vcs
  • debian chroot - it's just classic plus a function that I don't think is used much (and if it is it should be added to the others as well)
  • lonetwin and screen savvy because they are just slight variations on classic
  • justadollar because it's unhelpful and easy to build yourself

and performs some light cleanup on the others. It renames the prompts named after people who did not contribute them directly to fish.

Work on #7884.

TODOs:

  • [N/A] Changes to fish usage are reflected in user documentation/manpages.
  • [N/A] Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

@faho faho added this to the fish 3.3.0 milestone Apr 3, 2021
@faho
Copy link
Member Author

faho commented Apr 3, 2021

Screenshot of all the prompts after this:

Screenshot_20210403_110947

We are now down to 10, from 16. Note that "informative" doesn't abbreviate directories by default (but it does use prompt_pwd).

@ridiculousfish
Copy link
Member

Thanks for tackling this. Lots of good cleanup here.

I think we should have at least one simple prompt (not necessarily justadollar, but similarly simple). Some users may prefer a simple prompt, and don't want to learn how to build it; or they can use the simple prompt as a starting point to build from.

@faho
Copy link
Member Author

faho commented Apr 4, 2021

I think we should have at least one simple prompt (not necessarily justadollar, but similarly simple)

Sure, but I would argue that minimalist is much better in that role. Let me quote the full source:

# name: Minimalist
# author: ridiculous_fish

function fish_prompt
    set_color $fish_color_cwd
    echo -n (basename $PWD)
    set_color normal
    echo -n ' ) '
end

My fear is that justadollar is confusing to people, because it doesn't even contain $PWD. It's also not good as a starting point because it doesn't even show that $PWD exists, or that set_color is a thing.

By contrast going from minimalist to something justadollar-like is just removing the first three (or two) lines, which should be intuitively understandable.

Not that I'm married to minimalist in particular - I don't love showing only the last component of $PWD. My preferred "very simple" prompt would be something like echo -n (prompt_pwd) '$' (or maybe with color).

But I would quite like to remove justadollar because it's a disservice to those who pick it accidentally.

@faho
Copy link
Member Author

faho commented Apr 4, 2021

Alright, I renamed the Classic+VCS prompt to "Default" and added a "Simple" prompt that's the baseline I would expect - it's basically what you get when you look for a nice default $PS1 for bash:

function fish_prompt
    # This is a simple prompt. It looks like
    # alfa@nobby /path/to/dir $
    # with the path shortened and colored
    # and a "#" instead of a "$" when run as root.
    set -l symbol ' $ '
    set -l color $fish_color_cwd
    if fish_is_root_user
        set symbol ' # '
        set -q fish_color_cwd_root
        and set color $fish_color_cwd_root
    end

    echo -n $USER@$hostname

    set_color $color
    echo -n (prompt_pwd)
    set_color normal

    echo -n $symbol
end

Once you've seen this, you should be able to figure out how to make all sorts of prompts.

@krobelus
Copy link
Member

krobelus commented Apr 4, 2021

I appreciate quiet prompts; the $ one is nice when sending terminal contents to someone else because people recognize the dollar as prompt, and other prompt information is usually not relevant. But I agree it need not be in our samples.

I'd shave off the fish_is_root_user part of the simple prompt, to make it even simpler.
Or is it normal for people to copy over their prompt to root as well?

@faho
Copy link
Member Author

faho commented Apr 4, 2021

I'd shave off the fish_is_root_user part of the simple prompt, to make it even simpler.
Or is it normal for people to copy over their prompt to root as well?

This is what \$ in $PS1 in bash does. The default $PS1 on arch is [\u@\h \W]\$, so it does this (and also the weird thing of only printing the basename for $PWD).

This is another one of those things that are most useful when they are the default - people don't tend to customize root much, but when they pick a prompt from fish_config it should still make clear that it is root.

We could probably get by with adding a helper function called prompt_symbol. Takes two args for the user symbol and the root symbol, and then calls fish_is_root_user and prints the corresponding symbol. That doesn't do the PWD color, however, but we could also leave it out for this prompt. Either leave it at $fish_color_cwd or don't color it, or hard-code a different color.

@zanchey
Copy link
Member

zanchey commented Apr 6, 2021

#1351 re Debian chroot prompt

@faho
Copy link
Member Author

faho commented Apr 7, 2021

#1351 re Debian chroot prompt

So the argumentation there was that we don't want to make the default more complicated, so it should be in a separate prompt.

I disagree pretty strongly with that, because, like I've said numerous times: This is most useful when it's the default. When you need to specifically configure it you most likely won't, and then you'll never see it.

It's also a super cheesy solution because it is specific to debian. Any good solution here should probably also work for other kinds of virtualization.

I would argue for:

  • Make a "machine id" function
  • Add it to the default prompt

I've argued for this in the past with #6398. Note also that Debian apparently defaults to showing chroots for bash.

I don't think a separate prompt is of much use.

faho added 9 commits April 8, 2021 11:12
The "classic" prompts are all just variations on a theme, let's just
keep the default classic+vcs.

"Justadollar" is very unlikely to be what you want and also trivial to
write yourself.

I have no idea what screen_savvy even is for - it reacts to "$WINDOW",
but I don't know anything that even uses that variable.

Lonetwin is just unremarkable, and the debian chroot prompt has one special feature that should be integrated into the other prompts.
Some of these are just the git prompt defaults anyway, so remove them here.
`git status` will descend the entire repo, which is *slooooow*
So we don't just check for "root"
Unless that person directly contributed the prompt.

We name them after a feature - the Scales prompt feature a ">>>" which
kinda looks like fish scales, the "Arrow" prompt starts with a
prominent "➜".

Naming them after people looks like an endorsement of that particular
person, and like they are someone to look up to, especially when they
aren't involved with the project.

The "terlar" and "acidhub" prompts stay for now because they
contributed the prompt themselves, they are also much less prominent.
This would, with the default color, have an ugly red background.

So just remove the space.
That's what it is, and without the "classic" prompt to compare it
doesn't make any sense anymore.
This should be a simple prompt that doesn't place a huge strain on the
system but communicates the most important information simply and
effectively.

It should be a good jumping off point for making your own prompt.
@faho faho merged commit 4239ba1 into fish-shell:master Apr 8, 2021
@faho
Copy link
Member Author

faho commented Apr 8, 2021

Alright. The perfect being the enemy of the good, I have now merged this.

It doesn't solve all issues with the sample prompts, but it is an improvement.

@faho faho deleted the sample-prompts branch July 29, 2021 15:57
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants