overhaul shell_integration to enable individual control over ansi escape sequences#12629
Conversation
|
Thanks for this one Darren. |
|
With this PR, confirmed that |
|
@rgwood So, you're saying that osc633 is being activated when NOT inside the vscode terminal? If that's the case then it's a bug. It should only be available within the vscode terminal and it determines this by checking the env var TERM_PROGRAM for "vscode". However, osc133 has always affected wezterm and should be what is used outside of the vscode terminal. |
|
With escape-artist, I don't see osc633 being sent if it's off, but I do see other problems. Thanks! |
I think so? Setting |
|
I did that too but it was sending 133 not 633, so that was a bug. I also found some logic issues that I'm trying to fix now. I'm building release to test with escape-artist. :) |
# Description This PR is a continuation of #12629 and meant to address [Reilly's stated issue](#12629 (comment)). With this PR, nushell should work more consistently with WezTerm on Windows. However, that means continued scrolling with typing if osc133 is enabled. If it's possible to run WezTerm inside of vscode, then having osc633 enabled will also cause the display to scroll with every character typed. I think the cause of this is that reedline paints the entire prompt on each character typed. We need to figure out how to fix that, but that's in reedline. For my purposes, I keep osc133 and osc633 set to true and don't use WezTerm on Windows. Thanks @rgwood for reporting the issue. I found several logic errors. It's often good to come back to PRs and look at them with fresh eyes. I think this is pretty close to logically correct now. However, I'm approaching burn out on ansi escape codes so i could've missed something. Kudos to [escape-artist](https://github.com/rgwood/escape-artist) for helping me debug an ansi escape codes that are actually being sent to the terminal. It was an invaluable tool. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> # Tests + Formatting <!-- 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 > ``` --> # After Submitting <!-- 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. -->
# Description This PR is a continuation of nushell#12629 and meant to address [Reilly's stated issue](nushell#12629 (comment)). With this PR, nushell should work more consistently with WezTerm on Windows. However, that means continued scrolling with typing if osc133 is enabled. If it's possible to run WezTerm inside of vscode, then having osc633 enabled will also cause the display to scroll with every character typed. I think the cause of this is that reedline paints the entire prompt on each character typed. We need to figure out how to fix that, but that's in reedline. For my purposes, I keep osc133 and osc633 set to true and don't use WezTerm on Windows. Thanks @rgwood for reporting the issue. I found several logic errors. It's often good to come back to PRs and look at them with fresh eyes. I think this is pretty close to logically correct now. However, I'm approaching burn out on ansi escape codes so i could've missed something. Kudos to [escape-artist](https://github.com/rgwood/escape-artist) for helping me debug an ansi escape codes that are actually being sent to the terminal. It was an invaluable tool. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> # Tests + Formatting <!-- 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 > ``` --> # After Submitting <!-- 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. -->
Any reason to make this a breaking change? Would it not have been possible to interpret |
|
yup, that would've been possible. I didn't think of it. |
|
Adding a comment here for noobs like me that didn't knew how to apply the suggested tip;
This is an override of the default file |
|
Since $env.config.shell_integration.osc133 = false(The config error recovery will recover the rest of the record values, but its both terser and more realistic in that it just sets the value you care about and doesn't imply clearing the other keys, which doesn't occurr) |
Description
This PR overhauls the shell_integration system by allowing individual control over which ansi escape sequences are used. As we continue to broaden our support for more ansi escape sequences, we can't really have an all-or-nothing strategy. Some ansi escapes cause problems in certain operating systems or terminals. We should allow the user to choose which escapes they want.
TODO:
closes #11301
User-Facing Changes
shell_integration is no longer a boolean value. This is what is supported in the default_config.nu
Tests + Formatting
After Submitting