nushell/crates/nu-cli/src/prompt_update.rs

278 lines
9.3 KiB
Rust
Raw Normal View History

use crate::NushellPrompt;
use log::trace;
use nu_engine::eval_subexpression;
use nu_protocol::report_error;
2022-01-18 08:48:28 +00:00
use nu_protocol::{
engine::{EngineState, Stack, StateWorkingSet},
Config, PipelineData, Value,
2022-01-18 08:48:28 +00:00
};
use reedline::Prompt;
use std::borrow::Cow;
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
use std::sync::{Arc, RwLock};
2022-01-18 08:48:28 +00:00
// Name of environment variable where the prompt could be stored
pub(crate) const PROMPT_COMMAND: &str = "PROMPT_COMMAND";
pub(crate) const PROMPT_COMMAND_RIGHT: &str = "PROMPT_COMMAND_RIGHT";
pub(crate) const PROMPT_INDICATOR: &str = "PROMPT_INDICATOR";
pub(crate) const PROMPT_INDICATOR_VI_INSERT: &str = "PROMPT_INDICATOR_VI_INSERT";
2022-01-27 07:53:23 +00:00
pub(crate) const PROMPT_INDICATOR_VI_NORMAL: &str = "PROMPT_INDICATOR_VI_NORMAL";
2022-01-18 08:48:28 +00:00
pub(crate) const PROMPT_MULTILINE_INDICATOR: &str = "PROMPT_MULTILINE_INDICATOR";
Transient prompt (#10391) ## Description This PR uses environment variables to enable and set a transient prompt, which lets you draw a different prompt once you've entered a command and you've moved on to the next line. This is useful if you have a fancy two-line prompt with a bunch of info about time and git status that you don't really need in your scrollback buffer. Here's a screenshot. You can see how my usual prompt has two lines and would take up a lot more space if every past command also used the full prompt, but reducing past prompts to `🚀` or `>` makes it take up less space. ![image](https://github.com/nushell/nushell/assets/45539777/dde8d0f5-f95f-4529-9a14-b7919bd51126) I added the following lines to my `env.nu` to get that rocket as the prompt initially: ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| "" } $env.TRANSIENT_PROMPT_INDICATOR = {|| open --raw "~/.prompt-indicator" } $env.TRANSIENT_PROMPT_INDICATOR_VI_INSERT = $env.TRANSIENT_PROMPT_INDICATOR ``` ## User-Facing Changes If you want to change a segment of the prompt, set the corresponding `TRANSIENT_PROMPT_*` variable. <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --> ## Problems/Things to Consider: - The transient prompt clones the `Stack` at the very beginning of the session and keeps that around. I'm not sure if that could cause problems, but if so, it could probably take an `Arc<State>` instead. - This isn't truly a problem, but now there's even more environment variables, which is kinda annoying. - There might be some performance issues with creating a new `NushellPrompt` object and cloning the `Stack` for every segment of the transient prompt. What's more, the transient prompt is added to the `Reedline` object whether or not the user has enabled transient prompt, so if there are indeed performance issues, simply disabling the transient prompt won't help. - Perhaps instead of a separate `TRANSIENT_PROMPT_INDICATOR_VI_INSERT` and `TRANSIENT_PROMPT_INDICATOR_VI_NORMAL`, `TRANSIENT_PROMPT_INDICATOR` could be used for both (if it exists). Insert and normal mode don't really matter for previously entered commands.
2023-09-22 19:35:09 +00:00
pub(crate) const TRANSIENT_PROMPT_COMMAND: &str = "TRANSIENT_PROMPT_COMMAND";
pub(crate) const TRANSIENT_PROMPT_COMMAND_RIGHT: &str = "TRANSIENT_PROMPT_COMMAND_RIGHT";
pub(crate) const TRANSIENT_PROMPT_INDICATOR: &str = "TRANSIENT_PROMPT_INDICATOR";
pub(crate) const TRANSIENT_PROMPT_INDICATOR_VI_INSERT: &str =
"TRANSIENT_PROMPT_INDICATOR_VI_INSERT";
pub(crate) const TRANSIENT_PROMPT_INDICATOR_VI_NORMAL: &str =
"TRANSIENT_PROMPT_INDICATOR_VI_NORMAL";
pub(crate) const TRANSIENT_PROMPT_MULTILINE_INDICATOR: &str =
"TRANSIENT_PROMPT_MULTILINE_INDICATOR";
// According to Daniel Imms @Tyriar, we need to do these this way:
// <133 A><prompt><133 B><command><133 C><command output>
const PRE_PROMPT_MARKER: &str = "\x1b]133;A\x1b\\";
const POST_PROMPT_MARKER: &str = "\x1b]133;B\x1b\\";
2022-01-18 08:48:28 +00:00
fn get_prompt_string(
prompt: &str,
config: &Config,
engine_state: &EngineState,
stack: &mut Stack,
) -> Option<String> {
stack
.get_env_var(engine_state, prompt)
.and_then(|v| match v {
Value::Closure { val, .. } => {
let block = engine_state.get_block(val.block_id);
let mut stack = stack.captures_to_stack(val.captures);
// Use eval_subexpression to force a redirection of output, so we can use everything in prompt
let ret_val =
eval_subexpression(engine_state, &mut stack, block, PipelineData::empty());
trace!(
"get_prompt_string (block) {}:{}:{}",
file!(),
line!(),
column!()
);
2023-01-21 13:47:00 +00:00
ret_val
.map_err(|err| {
let working_set = StateWorkingSet::new(engine_state);
report_error(&working_set, &err);
2023-01-21 13:47:00 +00:00
})
.ok()
}
Value::Block { val: block_id, .. } => {
let block = engine_state.get_block(block_id);
// Use eval_subexpression to force a redirection of output, so we can use everything in prompt
let ret_val = eval_subexpression(engine_state, stack, block, PipelineData::empty());
trace!(
"get_prompt_string (block) {}:{}:{}",
file!(),
line!(),
column!()
);
2023-01-21 13:47:00 +00:00
ret_val
.map_err(|err| {
let working_set = StateWorkingSet::new(engine_state);
report_error(&working_set, &err);
2023-01-21 13:47:00 +00:00
})
.ok()
}
Value::String { .. } => Some(PipelineData::Value(v.clone(), None)),
_ => None,
2022-01-18 08:48:28 +00:00
})
.and_then(|pipeline_data| {
let output = pipeline_data.collect_string("", config).ok();
output.map(|mut x| {
// Just remove the very last newline.
if x.ends_with('\n') {
x.pop();
}
if x.ends_with('\r') {
x.pop();
}
x
})
})
2022-01-18 08:48:28 +00:00
}
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
pub(crate) fn update_prompt(
2022-01-18 08:48:28 +00:00
config: &Config,
engine_state: &EngineState,
stack: &Stack,
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
nu_prompt: Arc<RwLock<NushellPrompt>>,
) {
2022-01-18 08:48:28 +00:00
let mut stack = stack.clone();
let left_prompt_string = get_prompt_string(PROMPT_COMMAND, config, engine_state, &mut stack);
// Now that we have the prompt string lets ansify it.
// <133 A><prompt><133 B><command><133 C><command output>
let left_prompt_string = if config.shell_integration {
if let Some(prompt_string) = left_prompt_string {
Some(format!(
"{PRE_PROMPT_MARKER}{prompt_string}{POST_PROMPT_MARKER}"
))
} else {
left_prompt_string
}
} else {
left_prompt_string
};
let right_prompt_string =
get_prompt_string(PROMPT_COMMAND_RIGHT, config, engine_state, &mut stack);
let prompt_indicator_string =
get_prompt_string(PROMPT_INDICATOR, config, engine_state, &mut stack);
let prompt_multiline_string =
get_prompt_string(PROMPT_MULTILINE_INDICATOR, config, engine_state, &mut stack);
let prompt_vi_insert_string =
get_prompt_string(PROMPT_INDICATOR_VI_INSERT, config, engine_state, &mut stack);
let prompt_vi_normal_string =
get_prompt_string(PROMPT_INDICATOR_VI_NORMAL, config, engine_state, &mut stack);
2022-01-18 08:48:28 +00:00
// apply the other indicators
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
nu_prompt
.write()
.expect("Could not lock on nu_prompt to update")
.update_all_prompt_strings(
left_prompt_string,
right_prompt_string,
prompt_indicator_string,
prompt_multiline_string,
(prompt_vi_insert_string, prompt_vi_normal_string),
config.render_right_prompt_on_last_line,
);
2022-01-18 08:48:28 +00:00
trace!("update_prompt {}:{}:{}", file!(), line!(), column!());
2022-01-18 08:48:28 +00:00
}
Transient prompt (#10391) ## Description This PR uses environment variables to enable and set a transient prompt, which lets you draw a different prompt once you've entered a command and you've moved on to the next line. This is useful if you have a fancy two-line prompt with a bunch of info about time and git status that you don't really need in your scrollback buffer. Here's a screenshot. You can see how my usual prompt has two lines and would take up a lot more space if every past command also used the full prompt, but reducing past prompts to `🚀` or `>` makes it take up less space. ![image](https://github.com/nushell/nushell/assets/45539777/dde8d0f5-f95f-4529-9a14-b7919bd51126) I added the following lines to my `env.nu` to get that rocket as the prompt initially: ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| "" } $env.TRANSIENT_PROMPT_INDICATOR = {|| open --raw "~/.prompt-indicator" } $env.TRANSIENT_PROMPT_INDICATOR_VI_INSERT = $env.TRANSIENT_PROMPT_INDICATOR ``` ## User-Facing Changes If you want to change a segment of the prompt, set the corresponding `TRANSIENT_PROMPT_*` variable. <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --> ## Problems/Things to Consider: - The transient prompt clones the `Stack` at the very beginning of the session and keeps that around. I'm not sure if that could cause problems, but if so, it could probably take an `Arc<State>` instead. - This isn't truly a problem, but now there's even more environment variables, which is kinda annoying. - There might be some performance issues with creating a new `NushellPrompt` object and cloning the `Stack` for every segment of the transient prompt. What's more, the transient prompt is added to the `Reedline` object whether or not the user has enabled transient prompt, so if there are indeed performance issues, simply disabling the transient prompt won't help. - Perhaps instead of a separate `TRANSIENT_PROMPT_INDICATOR_VI_INSERT` and `TRANSIENT_PROMPT_INDICATOR_VI_NORMAL`, `TRANSIENT_PROMPT_INDICATOR` could be used for both (if it exists). Insert and normal mode don't really matter for previously entered commands.
2023-09-22 19:35:09 +00:00
struct TransientPrompt {
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
prompt_lock: Arc<RwLock<NushellPrompt>>,
engine_state: Arc<EngineState>,
stack: Stack,
}
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
impl TransientPrompt {
fn new_prompt(&self) -> NushellPrompt {
if let Ok(prompt) = self.prompt_lock.read() {
prompt.clone()
} else {
NushellPrompt::new()
}
}
}
Transient prompt (#10391) ## Description This PR uses environment variables to enable and set a transient prompt, which lets you draw a different prompt once you've entered a command and you've moved on to the next line. This is useful if you have a fancy two-line prompt with a bunch of info about time and git status that you don't really need in your scrollback buffer. Here's a screenshot. You can see how my usual prompt has two lines and would take up a lot more space if every past command also used the full prompt, but reducing past prompts to `🚀` or `>` makes it take up less space. ![image](https://github.com/nushell/nushell/assets/45539777/dde8d0f5-f95f-4529-9a14-b7919bd51126) I added the following lines to my `env.nu` to get that rocket as the prompt initially: ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| "" } $env.TRANSIENT_PROMPT_INDICATOR = {|| open --raw "~/.prompt-indicator" } $env.TRANSIENT_PROMPT_INDICATOR_VI_INSERT = $env.TRANSIENT_PROMPT_INDICATOR ``` ## User-Facing Changes If you want to change a segment of the prompt, set the corresponding `TRANSIENT_PROMPT_*` variable. <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --> ## Problems/Things to Consider: - The transient prompt clones the `Stack` at the very beginning of the session and keeps that around. I'm not sure if that could cause problems, but if so, it could probably take an `Arc<State>` instead. - This isn't truly a problem, but now there's even more environment variables, which is kinda annoying. - There might be some performance issues with creating a new `NushellPrompt` object and cloning the `Stack` for every segment of the transient prompt. What's more, the transient prompt is added to the `Reedline` object whether or not the user has enabled transient prompt, so if there are indeed performance issues, simply disabling the transient prompt won't help. - Perhaps instead of a separate `TRANSIENT_PROMPT_INDICATOR_VI_INSERT` and `TRANSIENT_PROMPT_INDICATOR_VI_NORMAL`, `TRANSIENT_PROMPT_INDICATOR` could be used for both (if it exists). Insert and normal mode don't really matter for previously entered commands.
2023-09-22 19:35:09 +00:00
impl Prompt for TransientPrompt {
fn render_prompt_left(&self) -> Cow<str> {
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
let mut nu_prompt = self.new_prompt();
let config = &self.engine_state.get_config().clone();
let mut stack = self.stack.clone();
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
if let Some(s) = get_prompt_string(
TRANSIENT_PROMPT_COMMAND,
config,
&self.engine_state,
&mut stack,
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
) {
nu_prompt.update_prompt_left(Some(s))
}
nu_prompt.render_prompt_left().to_string().into()
}
Transient prompt (#10391) ## Description This PR uses environment variables to enable and set a transient prompt, which lets you draw a different prompt once you've entered a command and you've moved on to the next line. This is useful if you have a fancy two-line prompt with a bunch of info about time and git status that you don't really need in your scrollback buffer. Here's a screenshot. You can see how my usual prompt has two lines and would take up a lot more space if every past command also used the full prompt, but reducing past prompts to `🚀` or `>` makes it take up less space. ![image](https://github.com/nushell/nushell/assets/45539777/dde8d0f5-f95f-4529-9a14-b7919bd51126) I added the following lines to my `env.nu` to get that rocket as the prompt initially: ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| "" } $env.TRANSIENT_PROMPT_INDICATOR = {|| open --raw "~/.prompt-indicator" } $env.TRANSIENT_PROMPT_INDICATOR_VI_INSERT = $env.TRANSIENT_PROMPT_INDICATOR ``` ## User-Facing Changes If you want to change a segment of the prompt, set the corresponding `TRANSIENT_PROMPT_*` variable. <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --> ## Problems/Things to Consider: - The transient prompt clones the `Stack` at the very beginning of the session and keeps that around. I'm not sure if that could cause problems, but if so, it could probably take an `Arc<State>` instead. - This isn't truly a problem, but now there's even more environment variables, which is kinda annoying. - There might be some performance issues with creating a new `NushellPrompt` object and cloning the `Stack` for every segment of the transient prompt. What's more, the transient prompt is added to the `Reedline` object whether or not the user has enabled transient prompt, so if there are indeed performance issues, simply disabling the transient prompt won't help. - Perhaps instead of a separate `TRANSIENT_PROMPT_INDICATOR_VI_INSERT` and `TRANSIENT_PROMPT_INDICATOR_VI_NORMAL`, `TRANSIENT_PROMPT_INDICATOR` could be used for both (if it exists). Insert and normal mode don't really matter for previously entered commands.
2023-09-22 19:35:09 +00:00
fn render_prompt_right(&self) -> Cow<str> {
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
let mut nu_prompt = self.new_prompt();
let config = &self.engine_state.get_config().clone();
let mut stack = self.stack.clone();
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
if let Some(s) = get_prompt_string(
TRANSIENT_PROMPT_COMMAND_RIGHT,
config,
&self.engine_state,
&mut stack,
) {
nu_prompt.update_prompt_right(Some(s), config.render_right_prompt_on_last_line)
}
nu_prompt.render_prompt_right().to_string().into()
}
Transient prompt (#10391) ## Description This PR uses environment variables to enable and set a transient prompt, which lets you draw a different prompt once you've entered a command and you've moved on to the next line. This is useful if you have a fancy two-line prompt with a bunch of info about time and git status that you don't really need in your scrollback buffer. Here's a screenshot. You can see how my usual prompt has two lines and would take up a lot more space if every past command also used the full prompt, but reducing past prompts to `🚀` or `>` makes it take up less space. ![image](https://github.com/nushell/nushell/assets/45539777/dde8d0f5-f95f-4529-9a14-b7919bd51126) I added the following lines to my `env.nu` to get that rocket as the prompt initially: ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| "" } $env.TRANSIENT_PROMPT_INDICATOR = {|| open --raw "~/.prompt-indicator" } $env.TRANSIENT_PROMPT_INDICATOR_VI_INSERT = $env.TRANSIENT_PROMPT_INDICATOR ``` ## User-Facing Changes If you want to change a segment of the prompt, set the corresponding `TRANSIENT_PROMPT_*` variable. <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --> ## Problems/Things to Consider: - The transient prompt clones the `Stack` at the very beginning of the session and keeps that around. I'm not sure if that could cause problems, but if so, it could probably take an `Arc<State>` instead. - This isn't truly a problem, but now there's even more environment variables, which is kinda annoying. - There might be some performance issues with creating a new `NushellPrompt` object and cloning the `Stack` for every segment of the transient prompt. What's more, the transient prompt is added to the `Reedline` object whether or not the user has enabled transient prompt, so if there are indeed performance issues, simply disabling the transient prompt won't help. - Perhaps instead of a separate `TRANSIENT_PROMPT_INDICATOR_VI_INSERT` and `TRANSIENT_PROMPT_INDICATOR_VI_NORMAL`, `TRANSIENT_PROMPT_INDICATOR` could be used for both (if it exists). Insert and normal mode don't really matter for previously entered commands.
2023-09-22 19:35:09 +00:00
fn render_prompt_indicator(&self, prompt_mode: reedline::PromptEditMode) -> Cow<str> {
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
let mut nu_prompt = self.new_prompt();
let config = &self.engine_state.get_config().clone();
let mut stack = self.stack.clone();
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
if let Some(s) = get_prompt_string(
TRANSIENT_PROMPT_INDICATOR,
config,
&self.engine_state,
&mut stack,
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
) {
nu_prompt.update_prompt_indicator(Some(s))
}
if let Some(s) = get_prompt_string(
TRANSIENT_PROMPT_INDICATOR_VI_INSERT,
config,
&self.engine_state,
&mut stack,
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
) {
nu_prompt.update_prompt_vi_insert(Some(s))
}
if let Some(s) = get_prompt_string(
TRANSIENT_PROMPT_INDICATOR_VI_NORMAL,
config,
&self.engine_state,
&mut stack,
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
) {
nu_prompt.update_prompt_vi_normal(Some(s))
}
nu_prompt
.render_prompt_indicator(prompt_mode)
.to_string()
.into()
}
Transient prompt (#10391) ## Description This PR uses environment variables to enable and set a transient prompt, which lets you draw a different prompt once you've entered a command and you've moved on to the next line. This is useful if you have a fancy two-line prompt with a bunch of info about time and git status that you don't really need in your scrollback buffer. Here's a screenshot. You can see how my usual prompt has two lines and would take up a lot more space if every past command also used the full prompt, but reducing past prompts to `🚀` or `>` makes it take up less space. ![image](https://github.com/nushell/nushell/assets/45539777/dde8d0f5-f95f-4529-9a14-b7919bd51126) I added the following lines to my `env.nu` to get that rocket as the prompt initially: ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| "" } $env.TRANSIENT_PROMPT_INDICATOR = {|| open --raw "~/.prompt-indicator" } $env.TRANSIENT_PROMPT_INDICATOR_VI_INSERT = $env.TRANSIENT_PROMPT_INDICATOR ``` ## User-Facing Changes If you want to change a segment of the prompt, set the corresponding `TRANSIENT_PROMPT_*` variable. <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --> ## Problems/Things to Consider: - The transient prompt clones the `Stack` at the very beginning of the session and keeps that around. I'm not sure if that could cause problems, but if so, it could probably take an `Arc<State>` instead. - This isn't truly a problem, but now there's even more environment variables, which is kinda annoying. - There might be some performance issues with creating a new `NushellPrompt` object and cloning the `Stack` for every segment of the transient prompt. What's more, the transient prompt is added to the `Reedline` object whether or not the user has enabled transient prompt, so if there are indeed performance issues, simply disabling the transient prompt won't help. - Perhaps instead of a separate `TRANSIENT_PROMPT_INDICATOR_VI_INSERT` and `TRANSIENT_PROMPT_INDICATOR_VI_NORMAL`, `TRANSIENT_PROMPT_INDICATOR` could be used for both (if it exists). Insert and normal mode don't really matter for previously entered commands.
2023-09-22 19:35:09 +00:00
fn render_prompt_multiline_indicator(&self) -> Cow<str> {
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
let mut nu_prompt = self.new_prompt();
let config = &self.engine_state.get_config().clone();
let mut stack = self.stack.clone();
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
if let Some(s) = get_prompt_string(
TRANSIENT_PROMPT_MULTILINE_INDICATOR,
config,
&self.engine_state,
&mut stack,
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
) {
nu_prompt.update_prompt_multiline(Some(s))
}
nu_prompt
.render_prompt_multiline_indicator()
.to_string()
.into()
}
Transient prompt (#10391) ## Description This PR uses environment variables to enable and set a transient prompt, which lets you draw a different prompt once you've entered a command and you've moved on to the next line. This is useful if you have a fancy two-line prompt with a bunch of info about time and git status that you don't really need in your scrollback buffer. Here's a screenshot. You can see how my usual prompt has two lines and would take up a lot more space if every past command also used the full prompt, but reducing past prompts to `🚀` or `>` makes it take up less space. ![image](https://github.com/nushell/nushell/assets/45539777/dde8d0f5-f95f-4529-9a14-b7919bd51126) I added the following lines to my `env.nu` to get that rocket as the prompt initially: ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| "" } $env.TRANSIENT_PROMPT_INDICATOR = {|| open --raw "~/.prompt-indicator" } $env.TRANSIENT_PROMPT_INDICATOR_VI_INSERT = $env.TRANSIENT_PROMPT_INDICATOR ``` ## User-Facing Changes If you want to change a segment of the prompt, set the corresponding `TRANSIENT_PROMPT_*` variable. <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --> ## Problems/Things to Consider: - The transient prompt clones the `Stack` at the very beginning of the session and keeps that around. I'm not sure if that could cause problems, but if so, it could probably take an `Arc<State>` instead. - This isn't truly a problem, but now there's even more environment variables, which is kinda annoying. - There might be some performance issues with creating a new `NushellPrompt` object and cloning the `Stack` for every segment of the transient prompt. What's more, the transient prompt is added to the `Reedline` object whether or not the user has enabled transient prompt, so if there are indeed performance issues, simply disabling the transient prompt won't help. - Perhaps instead of a separate `TRANSIENT_PROMPT_INDICATOR_VI_INSERT` and `TRANSIENT_PROMPT_INDICATOR_VI_NORMAL`, `TRANSIENT_PROMPT_INDICATOR` could be used for both (if it exists). Insert and normal mode don't really matter for previously entered commands.
2023-09-22 19:35:09 +00:00
fn render_prompt_history_search_indicator(
&self,
history_search: reedline::PromptHistorySearch,
) -> Cow<str> {
NushellPrompt::new()
.render_prompt_history_search_indicator(history_search)
.to_string()
.into()
}
}
/// Construct the transient prompt
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
pub(crate) fn transient_prompt(
prompt_lock: Arc<RwLock<NushellPrompt>>,
engine_state: Arc<EngineState>,
stack: &Stack,
) -> Box<dyn Prompt> {
Box::new(TransientPrompt {
Don't redraw prompt when transient prompt disabled (#10788) # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> moonlander pointed out in Discord that the transient prompt feature added in release 0.86 (implemented in #10391) is causing the normal prompt to be redrawn when the transient prompt variables are unset or set to null. This PR is for fixing that, although it's more of a bandaid fix. Maybe the transient prompt feature should be taken out entirely for now so more thought can be given to its implementation. Previously, I'd thought that when reedline redraws the prompt after a command is entered, it's a whole new prompt, but apparently it's actually the same prompt as the current line (?). So now, `nu_prompt` in `repl.rs` is an `Arc<RwLock<NushellPrompt>>` (rather than just a `NushellPrompt`), and this `Arc` is shared with the `TransientPrompt` object so that if it can't find one of the `TRANSIENT_PROMPT_*` variables, it uses a segment from `NushellPrompt` rather than re-evaluate `PROMPT_COMMAND`, `PROMPT_COMMAND_RIGHT`, etc. Using an `RwLock` means that there's a bunch of `.expect()`s all over the place, which is not nice. It could perhaps be avoided with some changes on the reedline side. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> `$env.PROMPT_COMMAND` (and other such variables) should no longer be executed twice if the corresponding `$env.TRANSIENT_PROMPT_*` variable is not set. <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass (on Windows make sure to [enable developer mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging)) - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # Steps to reproduce Described by moonlander in Discord [here](https://discord.com/channels/601130461678272522/614593951969574961/1164928022126792844). Adding this to `env.nu` will result in `11` being added to `/tmp/run_count` every time any command is run. The expected behavior is a single `1` being added to `/tmp/run_count` instead of two. The prompt command should not be executed again when the prompt is redrawn after a command is executed. ```nu $env.PROMPT_COMMAND = {|| touch /tmp/run_count '1' | save /tmp/run_count --append '>' } # $env.TRANSIENT_PROMPT_COMMAND not set ``` If the following is added to `env.nu`, then `12` will be added to `/tmp/run_count` every time any command is run, which is expected behavior because the normal prompt command must be displayed the first time the prompt is shown, then the transient prompt command is run when the prompt is redrawn. ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| touch /tmp/run_count '2' | save /tmp/run_count --append '>' } ``` Here's a screenshot of what adding that first snippet looks like (`cargo run` in the `main` branch): ![image](https://github.com/nushell/nushell/assets/45539777/b27a5c07-55b4-43c7-8a2c-0deba2d9d53a) Here's a screenshot of what it looks like with this PR (only one `1` is added to `/tmp/run_count` each time): ![image](https://github.com/nushell/nushell/assets/45539777/2b5c0a3a-8566-4428-9fda-1ffcc1dd6ae3)
2023-12-15 19:41:44 +00:00
prompt_lock,
engine_state,
stack: stack.clone(),
})
Transient prompt (#10391) ## Description This PR uses environment variables to enable and set a transient prompt, which lets you draw a different prompt once you've entered a command and you've moved on to the next line. This is useful if you have a fancy two-line prompt with a bunch of info about time and git status that you don't really need in your scrollback buffer. Here's a screenshot. You can see how my usual prompt has two lines and would take up a lot more space if every past command also used the full prompt, but reducing past prompts to `🚀` or `>` makes it take up less space. ![image](https://github.com/nushell/nushell/assets/45539777/dde8d0f5-f95f-4529-9a14-b7919bd51126) I added the following lines to my `env.nu` to get that rocket as the prompt initially: ```nu $env.TRANSIENT_PROMPT_COMMAND = {|| "" } $env.TRANSIENT_PROMPT_INDICATOR = {|| open --raw "~/.prompt-indicator" } $env.TRANSIENT_PROMPT_INDICATOR_VI_INSERT = $env.TRANSIENT_PROMPT_INDICATOR ``` ## User-Facing Changes If you want to change a segment of the prompt, set the corresponding `TRANSIENT_PROMPT_*` variable. <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. --> ## Problems/Things to Consider: - The transient prompt clones the `Stack` at the very beginning of the session and keeps that around. I'm not sure if that could cause problems, but if so, it could probably take an `Arc<State>` instead. - This isn't truly a problem, but now there's even more environment variables, which is kinda annoying. - There might be some performance issues with creating a new `NushellPrompt` object and cloning the `Stack` for every segment of the transient prompt. What's more, the transient prompt is added to the `Reedline` object whether or not the user has enabled transient prompt, so if there are indeed performance issues, simply disabling the transient prompt won't help. - Perhaps instead of a separate `TRANSIENT_PROMPT_INDICATOR_VI_INSERT` and `TRANSIENT_PROMPT_INDICATOR_VI_NORMAL`, `TRANSIENT_PROMPT_INDICATOR` could be used for both (if it exists). Insert and normal mode don't really matter for previously entered commands.
2023-09-22 19:35:09 +00:00
}