(DON'T MERGE) feat: starship prompt#804
(DON'T MERGE) feat: starship prompt#804Southclaws wants to merge 6 commits intonushell:masterfrom Southclaws:starship-prompt
Conversation
Kind of touches on #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 #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.
|
Found some possible issues, maybe regressions caused by this PR but not sure. The issue is the prompt is being cut off by two characters then being replaced with Which, if you change the readline = rl.readline_with_initial("prompt", (&cmd, ""));Which then lead to noticing another bug, which may be caused by this one, where the caret seems to be one character to the left of the actual typing area - not sure what the actual terminology is but in the following image, I have already typed These might be related to how the rustyline Editor is configured. |
|
You may also need to change the prompt that is highlighted by rustyline: https://github.com/nushell/nushell/blob/master/src/shell/helper.rs#L59-L62 |
|
Ah, perfect! That clears up the confusion thanks. |
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.
|
From the most recent commit: 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 For now, I've simply stripped off the additional 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. |
|
I'll fix the remaining lint issues later, it might be simpler to just move the prompt code with feature conditions to a new file so the |
|
Don't merge, it's still a little broken! It's not passing the cwd to starship correctly for some reason... |
|
Changed title until you're ready :) (mostly so one one of us doesn't accidentally click merge just yet) |
|
I think github supports draft pull requests for this exact use case. |
|
Yeah it was a draft then the tests passed and I figured it was ready and now I can't switch it back to draft. |
|
@Southclaws no worries :) Just let us know when it's ready |
|
@Southclaws - oh, I had another question. I was playing with starship in Windows, and I noticed the default prompt has some unicode that the Windows terminal doesn't handle so well. Do you know if we can configure what the default prompt is when we land support into nushell? We can let users configure it from there but I figured we could make the default crossplatform-friendly. |
|
Okay so I found the answer to the issue I was running into with paths: the problem was, Starship is moving to using logical paths by default but right now there's a configuration option to force it. For some reason, not setting this results in the path not updating correctly when you move around directories. [directory]
use_logical_path = falseIs the Starship configuration solution. This should also be possible pragmatically but it might be best to wait until Starship does this by default. In regards to cross platform, I actually forgot about this when evaluating Starship (which is not like me at all!) and it seems it doesn't explicitly claim Windows support on the readme though it seems there are quite a few mentions of Windows in the issues and Discord so there are certainly users with it. It's worth noting that Starship itself requires a Powerline font, I know the new terminal is fine with these but not sure about the old PowerShell/Command Prompt emulators. But it seems like it's not going to be a huge blocker, the prompt itself isn't doing anything complicated and Linux-specific by the looks of it, at the end of the day it just writes some text to stdout! When I've got my Windows laptop again I will play around in the various terminal emulators and see what works and what doesn't. |
|
Okay so the Windows answer is good news! It's being worked on and is fairly close, this is the PR to track: starship/starship#470 |
|
@Southclaws - just noticed that starship/starship#470 has landed (which is great!). Are there steps we need to do on our side to get ready to add support for it not that it's landed? |
|
Yeah possibly, I will look into it this weekend. I was following that PR and it seems like it shouldn't affect this much. |
|
@Southclaws are there things we can help with? We're putting out 0.5.0 this next week, and it'd be fun to see if we could have starship support available. |
|
I've been super busy these last few weeks but that's slowing down now so I'll get some time to clean this up and finish it off soon! |




Kind of touches on #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 #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.