Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix rendering prompt after ExecuteHostCommand #758

Merged
merged 3 commits into from
Feb 27, 2024

Conversation

kit494way
Copy link
Contributor

Fix #755

@fdncred
Copy link
Collaborator

fdncred commented Feb 26, 2024

Do you think this PR will fix this nushell issue? nushell/nushell#11981

@kit494way
Copy link
Contributor Author

Yes, I think this PR will fix the issue.

Copy link
Member

@sholderbach sholderbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic looks good.
From a maintainability perspective a small comment but we should test this with Nushell

src/engine.rs Outdated Show resolved Hide resolved
kit494way and others added 2 commits February 27, 2024 22:53
Co-authored-by: Stefan Holderbach <[email protected]>
@kit494way
Copy link
Contributor Author

Thanks, I accepted the suggestion and renamed.

@fdncred
Copy link
Collaborator

fdncred commented Feb 27, 2024

yay! i was hoping we'd get this in before the next release. i'm wondering what impacts it will have on people's keybindings, meaning, if people are relying on a newline, it could be breaking something. let's find out!

@fdncred fdncred merged commit 4fd1295 into nushell:main Feb 27, 2024
6 checks passed
fdncred added a commit to nushell/nushell that referenced this pull request Feb 27, 2024
# Description

This PR bumps reedline to the latest main which has the
`executehostcommand` changes
nushell/reedline#758 which essentially allows
reedline/nushell to call `executehostcommand` in key bindings and
rewrite the commandline buffer without inserting a newline.

# 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.
-->
kik4444 pushed a commit to kik4444/nushell-fork that referenced this pull request Feb 28, 2024
# Description

This PR bumps reedline to the latest main which has the
`executehostcommand` changes
nushell/reedline#758 which essentially allows
reedline/nushell to call `executehostcommand` in key bindings and
rewrite the commandline buffer without inserting a newline.

# 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.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

After ExecuteHostCommand is handled, new prompt is rendered
3 participants