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

Starship prompt #970

Merged
merged 7 commits into from Nov 16, 2019
Merged

Starship prompt #970

merged 7 commits into from Nov 16, 2019

Conversation

sophiajt
Copy link
Member

Make starship prompt an optional feature for Nu. Builds on work by @Southclaws

Screenshot from 2019-11-17 06-17-27

Closes #804

Southclaws and others added 7 commits October 8, 2019 16:25
Kind of touches on nushell#356 by integrating the Starship prompt directly into the shell.

Not finished yet and has surfaced a potential bug in rustyline anyway. It depends on starship/starship#509 being merged so the Starship prompt can be used as a library.

I could have tackled nushell#356 completely and implemented a full custom prompt feature but I felt this was a simpler approach given that Starship is both written in Rust so shelling out isn't necessary and it already has a bunch of useful features built in.

However, I would understand if it would be preferable to just scrap integrating Starship directly and instead implement a custom prompt system which would facilitate simply shelling out to Starship.
This resolves a small integration issue that would make custom prompts problematic (if they are implemented). The approach was to use the highlighter implementation in Helper to insert colour codes to the prompt however it heavily relies on the prompt being in a specific format, ending with a `> ` sequence. However, this should really be the job of the prompt itself not the presentation layer.

For now, I've simply stripped off the additional `> ` characters and passed in just the prompt itself without slicing off the last two characters. I moved the `\x1b[m` control sequence to the prompt creation in `cli.rs` as this feels like the more logical home for controlling what the prompt looks like. I can think of better ways to do this in future but this should be a fine solution for now.

In future it would probably make sense to completely separate prompts (be it, internal or external) from this code so it can be configured as an isolated piece of code.
@gitpod-io
Copy link

gitpod-io bot commented Nov 16, 2019

@sophiajt sophiajt merged commit 17e8a5c into nushell:master Nov 16, 2019
@sophiajt sophiajt deleted the starship-prompt branch November 16, 2019 17:44
@sophiajt
Copy link
Member Author

@Southclaws - I know you had some additional things you wanted to add to make the experience even better from Nu, and I'm definitely 👍 for that. I just wanted to see if I could get your baseline working, and then we could grow from there.

@waldyrious
Copy link
Contributor

waldyrious commented Nov 27, 2019

I'm having trouble figuring out how to enable the starship prompt. The documentation only explains how to configure the starship prompt once it's enabled, not how to enable it. I'm using 0.6.0 installed via Homebrew.

@fdcds
Copy link

fdcds commented Nov 27, 2019

The Fedora build appears to have the same issue.

@Southclaws
Copy link
Contributor

It's behind a feature-flag starship-prompt, not sure if any builds ship with it enabled but if you build from source and enable that flag it should be present in the prompt.

@waldyrious
Copy link
Contributor

if you build from source and enable that flag

Would you be so kind as to show how that would look like, i.e. what commands or CLI options I would have to use to do so? As someone who is aware of Rust, but never worked directly with it, I'd greatly appreciate the extra guidance. 😄

@sophiajt
Copy link
Member Author

@waldyrious - I'll update the blog post with installation steps

@devurandom
Copy link

The Fedora package is built with cargo build --release --all-features: https://copr-dist-git.fedorainfracloud.org/cgit/atim/nushell/nushell.git/tree/nushell.spec?id=65be21c7d49cd67d9495d177125c76a4fd70e8c5

Shouldn't this include starship-prompt? I.e. shouldn't nu from the Fedora package always start with a Starship prompt?

@waldyrious
Copy link
Contributor

waldyrious commented Dec 18, 2019

I'll update the blog post with installation steps

I appreciate the update to the blog post, but the command seems to be slightly off. Instead of --feature starship, it should be --features starship-prompt, as shown e.g. in #1005. I sent a PR to fix it: https://github.com/nushell/blog/pull/16

I've also created a PR to Homebrew to enable the starship prompt there as well.

elferherrera pushed a commit to elferherrera/nushell that referenced this pull request Feb 7, 2022
kubouch pushed a commit that referenced this pull request Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants