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

Terminate REPL if not connected to tty input #6480

Merged
merged 2 commits into from Sep 5, 2022

Conversation

sholderbach
Copy link
Member

Description

If the standard input stream is not a TTY abort the REPL execution.

Solves a problem as the current REPL tries to be I/O fault tolerant and
would indefinetely fail when crossterm tries to handle the STDIN.

Fixes #6452

Tests

Make sure you've done the following:

  • Add tests that cover your changes, either in the command examples, the crate/tests folder, or in the /tests folder.
  • Try to think about corner cases and various ways how your changes could break. Cover them with tests.
  • If adding tests is not possible, please document in the PR body a minimal example with steps on how to reproduce so one can verify your change works.

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 --features=extra -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect to check that you're using the standard code style
  • cargo test --workspace --features=extra to check that all the tests pass

If the standard input stream is not a TTY abort the REPL execution.

Solves a problem as the current REPL tries to be IO fault tolerant and
would indefinetely fail when crossterm tries to handle the STDIN.

Fixes nushell#6452
@sholderbach
Copy link
Member Author

OK one questionable case:

grafik

Is there a reason to pipe in external STDIN on startup and expect the REPL to start off regularly?

@sholderbach
Copy link
Member Author

I tried to be more helpful in my error message. If execution from a piped in stream of commands is desired, it has to be implemented in a different place (main.rs and friends).

@sholderbach sholderbach merged commit 33e1120 into nushell:main Sep 5, 2022
@sholderbach sholderbach deleted the stop-ttyless branch September 5, 2022 11:33
@fdncred
Copy link
Collaborator

fdncred commented Sep 5, 2022

There is a --stdin flag on nu that someone asked about in discord recently. So, someone is piping stdin to nushell, but I'm not sure if this falls into this use case or not?

@sholderbach
Copy link
Member Author

That should be handled some where in the logic of main.rs, so I don't think that should lead to regular invocation of the REPL. (I don't know how tools like pexpect that mock user interaction will play with this change, but for valid use case I would expect that you provide a script.)

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.

High CPU usage when launched from file explorer
2 participants