Repository navigation
refactor(cli): replace hand-rolled terminal code with crossterm - #127
Draft
thebrandonlucas wants to merge 1 commit into
Draft
thebrandonlucas wants to merge 1 commit into
thebrandonlucas wants to merge 1 commit into
Conversation
The hidden API-key prompt reads keys in crossterm's raw mode instead of rpassword, and a guard turns raw mode off through crossterm, which restores the saved terminal modes when Ctrl-C cancels a read that cannot return. This removes the unsafe tcgetattr/tcsetattr exception, the rpassword dependency, and the Unix-only restore: pickers and the hidden prompt now restore the terminal on every platform. A typed Ctrl-C at the hidden prompt exits 130 with the same interruption message as before. The picker, live-watch, and status-line escape sequences come from crossterm commands instead of literal bytes; the bytes written are the same, except that home is now ESC[1;1H.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes DEV-2904
Problem
terminal.rskept an approvedunsafeexception:TerminalModescalledlibc::tcgetattrandlibc::tcsetattrto put echo back when Ctrl-C cancelled the hidden API-key prompt, because rpassword restores echo only when its read returns. The restore was Unix-only. The picker, live-watch, and status-line guards also wrote escape sequences as literal bytes, although crossterm, which the CLI already uses for the width and the pickers, has a command for each.Change
/dev/ttywith echo and signals offTerminalModessaves and restores termios withunsafelibc calls (Unix only)RawModeguard callscrossterm::terminal::disable_raw_mode, which restores the modes crossterm saved when raw mode went on. It covers the hidden prompt and inquire's pickers on every platformInterrupted before any resource change was submitted(exit 130), the same as the pickers\x1b[?2004l\x1b[?25h,\x1b[?1049h…,\x1b[H\x1b[2J,\r\x1b[2Kas literalsDisableBracketedPaste,cursor::Show/Hide,Enter/LeaveAlternateScreen,MoveTo,Clear, rendered through aterminal::ansihelperansi()usesCommand::write_ansi, notexecute!. On Windows,execute!can call the console API directly, which would reorder the sequence with the buffered bytes around it. The bytes written are unchanged, except that home isESC[1;1Hinstead ofESC[H.docs/quality.md.rpasswordis removed (−22 lines inCargo.lock). Thecrosstermentry now listseventsandbracketed-paste; inquire already turned both on, so the dependency graph does not change.unsafeandlibcinsrc/are now limited tocheck_ownerandO_NOFOLLOWinconfig.rs. The integration tests still use libc to open and inspect pseudo-terminals.User-visible, so there is a CHANGELOG line: the hidden prompt is written to stderr instead of
/dev/tty, Ctrl-U clears it, and the restore works on every platform.Try it on staging
Setup: Running against staging. This saves an API key under a throwaway account name and then logs it out.
Use the organization and environment from
context:At
Environment API key:, type a few characters and press Ctrl-C. Expect: nothing you typed is shown,Interrupted before any resource change was submittedappears,echo $?prints 130, and typing in the shell echoes again (nostty saneneeded).Run the command again and type
wrong, press Ctrl-U, type a real or dummy key, use Backspace once, and press Enter. Expect: the key is not shown, and the command reportsimported. Then remove it:voltage logout --account try-keyTo check the picker guard, run
voltage wallets list --account mewithout--org. If your login sees more than one organization, a picker opens. Press Ctrl-C there, and check that the shell still echoes and shows its cursor.Validation
cargo fmt --check,cargo clippy --locked --all-targets -- -D warnings,cargo machete --with-metadata(innix develop),cargo test --locked(200 unit, 150 integration), andpython3 scripts/check-coverage.pyall pass.type_hiddenkey handling (typing, Shift, Backspace, Ctrl-U, ignored Alt/Ctrl chords, arrows, and key releases; Enter, Ctrl-J, and Ctrl-C), the read-result mapping (secret, interruption, and the transport error for a terminal that cannot enter raw mode or be read), and theansibyte output.the_hidden_key_prompt_saves_the_edited_key_without_showing_it: a pseudo-terminal types a key with Ctrl-U and Backspace edits. It checks the saved key, that the terminal output never contains it, and that the terminal's local modes match their values before the run.disable_raw_modecall removed, all three hidden-prompt tests fail, so the guard is what restores the terminal.scripts/crap-report.sh, cargo-llvm-cov 0.8.5, knots 1.16.0): 849 of 1060 functions measured, 0 violations. The highest score is 18.10,wait_for_paymentinsrc/api.rs, which this PR does not change. The new functions scoreread_hidden_keys5.0,type_hidden4.0,read_hidden3.01, and the rest 1–2.Limits
style.rsstill writes the OSC 8 hyperlink sequence by hand, because neither crossterm 0.29 nor anstyle has a hyperlink command. Thewordmark.rsescapes are test assertions only.