From a705c1f07de2b8ec3ba4fe46377242f151b996c1 Mon Sep 17 00:00:00 2001 From: Jesse Luehrs Date: Wed, 8 Mar 2023 22:31:00 -0500 Subject: use callbacks for events rather than tracking counters --- CHANGELOG.md | 12 +++++++ examples/fuzz.rs | 21 +++--------- src/callbacks.rs | 13 ++++++++ src/lib.rs | 3 ++ src/parser.rs | 14 ++++++++ src/screen.rs | 92 +++------------------------------------------------- src/state.rs | 61 ++++++++++++++++++++++++++++++++++ src/term.rs | 20 ------------ tests/control.rs | 39 ++++++++++++---------- tests/escape.rs | 39 ++++++++++++---------- tests/helpers/mod.rs | 18 ---------- tests/init.rs | 2 -- 12 files changed, 156 insertions(+), 178 deletions(-) create mode 100644 src/callbacks.rs create mode 100644 src/state.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 51a7f97..477fc5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,18 @@ ## [Unreleased] +### Added + +* `Parser::process_cb`, which works the same as `Parser::process` except that + it calls callbacks during parsing when it finds a terminal escape which is + potentially useful but not something that affects the screen itself. + +### Removed + +* `Screen::bells_diff`, `Screen::audible_bell_count`, + `Screen::visual_bell_count`, and `Screen::errors` have been removed in favor + of the new callback api described above. + ### Changed * `Parser::set_size` and `Parser::set_scrollback` have been moved to methods diff --git a/examples/fuzz.rs b/examples/fuzz.rs index 576af2c..f025368 100644 --- a/examples/fuzz.rs +++ b/examples/fuzz.rs @@ -3,10 +3,9 @@ use std::io::Read as _; #[path = "../tests/helpers/mod.rs"] mod helpers; -fn check_full(vt_base: &vt100::Screen, empty: &vt100::Screen, idx: usize) { +fn check_full(vt_base: &vt100::Screen, idx: usize) { let mut input = vec![]; input.extend(vt_base.state_formatted()); - input.extend(vt_base.bells_diff(empty)); let mut vt_full = vt100::Parser::default(); vt_full.process(&input); assert!( @@ -24,7 +23,6 @@ fn check_diff_empty( ) { let mut input = vec![]; input.extend(vt_base.state_diff(empty)); - input.extend(vt_base.bells_diff(empty)); let mut vt_diff_empty = vt100::Parser::default(); vt_diff_empty.process(&input); assert!( @@ -39,12 +37,10 @@ fn check_diff( vt_base: &vt100::Screen, vt_diff: &mut vt100::Parser, prev: &vt100::Screen, - empty: &vt100::Screen, idx: usize, ) { let mut input = vec![]; input.extend(vt_base.state_diff(prev)); - input.extend(vt_base.bells_diff(empty)); vt_diff.process(&input); assert!( helpers::compare_screens(vt_diff.screen(), vt_base), @@ -54,7 +50,7 @@ fn check_diff( ); } -fn check_rows(vt_base: &vt100::Screen, empty: &vt100::Screen, idx: usize) { +fn check_rows(vt_base: &vt100::Screen, idx: usize) { let mut input = vec![]; let mut wrapped = false; for (idx, row) in vt_base.rows_formatted(0, 80).enumerate() { @@ -70,7 +66,6 @@ fn check_rows(vt_base: &vt100::Screen, empty: &vt100::Screen, idx: usize) { input.extend(&vt_base.attributes_formatted()); input.extend(&vt_base.input_mode_formatted()); input.extend(&vt_base.title_formatted()); - input.extend(&vt_base.bells_diff(empty)); let mut vt_rows = vt100::Parser::default(); vt_rows.process(&input); assert!( @@ -106,16 +101,10 @@ fn main() { while let Some(byte) = read_byte() { vt_base.process(&[byte]); - check_full(vt_base.screen(), &empty_screen, idx); + check_full(vt_base.screen(), idx); check_diff_empty(vt_base.screen(), &empty_screen, idx); - check_diff( - vt_base.screen(), - &mut vt_diff, - &prev_screen, - &empty_screen, - idx, - ); - check_rows(vt_base.screen(), &empty_screen, idx); + check_diff(vt_base.screen(), &mut vt_diff, &prev_screen, idx); + check_rows(vt_base.screen(), idx); prev_screen = vt_base.screen().clone(); idx += 1; diff --git a/src/callbacks.rs b/src/callbacks.rs new file mode 100644 index 0000000..256eb58 --- /dev/null +++ b/src/callbacks.rs @@ -0,0 +1,13 @@ +/// This trait is used with `Parser::process_cb` to handle extra escape +/// sequences that don't have an impact on the terminal screen directly. +pub trait Callbacks { + /// This callback is called when the terminal requests an audible bell + /// (typically with `^G`). + fn audible_bell(&mut self, _: &mut crate::Screen) {} + /// This callback is called when the terminal requests an visual bell + /// (typically with `\eg`). + fn visual_bell(&mut self, _: &mut crate::Screen) {} + /// This callback is called when the terminal receives invalid input + /// (such as an invalid UTF-8 character or an used control character). + fn error(&mut self, _: &mut crate::Screen) {} +} diff --git a/src/lib.rs b/src/lib.rs index ddea0cd..92256f2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -47,14 +47,17 @@ #![allow(clippy::type_complexity)] mod attrs; +mod callbacks; mod cell; mod grid; mod parser; mod row; mod screen; +mod state; mod term; pub use attrs::Color; +pub use callbacks::Callbacks; pub use cell::Cell; pub use parser::Parser; pub use screen::{MouseProtocolEncoding, MouseProtocolMode, Screen}; diff --git a/src/parser.rs b/src/parser.rs index 01ba019..10ebf10 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -27,6 +27,20 @@ impl Parser { } } + /// Processes the contents of the given byte string, and updates the + /// in-memory terminal state. Calls methods on the given `Callbacks` + /// object when relevant escape sequences are seen. + pub fn process_cb( + &mut self, + bytes: &[u8], + callbacks: &mut impl crate::callbacks::Callbacks, + ) { + let mut state = crate::state::State::new(&mut self.screen, callbacks); + for byte in bytes { + self.parser.advance(&mut state, *byte); + } + } + /// Returns a reference to a `Screen` object containing the terminal /// state. #[must_use] diff --git a/src/screen.rs b/src/screen.rs index fe4e5cc..9da62c7 100644 --- a/src/screen.rs +++ b/src/screen.rs @@ -75,11 +75,6 @@ pub struct Screen { modes: u8, mouse_protocol_mode: MouseProtocolMode, mouse_protocol_encoding: MouseProtocolEncoding, - - audible_bell_count: usize, - visual_bell_count: usize, - - errors: usize, } impl Screen { @@ -102,11 +97,6 @@ impl Screen { modes: 0, mouse_protocol_mode: MouseProtocolMode::default(), mouse_protocol_encoding: MouseProtocolEncoding::default(), - - audible_bell_count: 0, - visual_bell_count: 0, - - errors: 0, } } @@ -256,15 +246,13 @@ impl Screen { /// Return escape codes sufficient to turn the terminal state of the /// screen `prev` into the current terminal state. This is a convenience - /// wrapper around `contents_diff`, `input_mode_diff`, `title_diff`, and - /// `bells_diff`. + /// wrapper around `contents_diff`, `input_mode_diff`, and `title_diff` #[must_use] pub fn state_diff(&self, prev: &Self) -> Vec { let mut contents = vec![]; self.write_contents_diff(&mut contents, prev); self.write_input_mode_diff(&mut contents, prev); self.write_title_diff(&mut contents, prev); - self.write_bells_diff(&mut contents, prev); contents } @@ -511,25 +499,6 @@ impl Screen { .write_buf(contents); } - /// Returns terminal escape sequences sufficient to cause audible and - /// visual bells to occur if they have been received since the terminal - /// described by `prev`. - #[must_use] - pub fn bells_diff(&self, prev: &Self) -> Vec { - let mut contents = vec![]; - self.write_bells_diff(&mut contents, prev); - contents - } - - fn write_bells_diff(&self, contents: &mut Vec, prev: &Self) { - if self.audible_bell_count != prev.audible_bell_count { - crate::term::AudibleBell::default().write_buf(contents); - } - if self.visual_bell_count != prev.visual_bell_count { - crate::term::VisualBell::default().write_buf(contents); - } - } - /// Returns terminal escape sequences sufficient to set the current /// terminal's drawing attributes. /// @@ -632,42 +601,6 @@ impl Screen { &self.icon_name } - /// Returns a value which changes every time an audible bell is received. - /// - /// Typically you would store this number after each call to `process`, - /// and trigger an audible bell whenever it changes. - /// - /// You shouldn't rely on the exact value returned here, since the exact - /// value will not be maintained by `contents_formatted` or - /// `contents_diff`. - #[must_use] - pub fn audible_bell_count(&self) -> usize { - self.audible_bell_count - } - - /// Returns a value which changes every time an visual bell is received. - /// - /// Typically you would store this number after each call to `process`, - /// and trigger an visual bell whenever it changes. - /// - /// You shouldn't rely on the exact value returned here, since the exact - /// value will not be maintained by `contents_formatted` or - /// `contents_diff`. - #[must_use] - pub fn visual_bell_count(&self) -> usize { - self.visual_bell_count - } - - /// Returns the number of parsing errors seen so far. - /// - /// Currently this only tracks invalid UTF-8 and control characters other - /// than `0x07`-`0x0f`. This can give an idea of whether the input stream - /// being fed to the parser is reasonable or not. - #[must_use] - pub fn errors(&self) -> usize { - self.errors - } - /// Returns whether the alternate screen is currently in use. #[must_use] pub fn alternate_screen(&self) -> bool { @@ -1061,10 +994,6 @@ impl Screen { // control codes - fn bel(&mut self) { - self.audible_bell_count += 1; - } - fn bs(&mut self) { self.grid_mut().col_dec(1); } @@ -1120,22 +1049,11 @@ impl Screen { fn ris(&mut self) { let title = self.title.clone(); let icon_name = self.icon_name.clone(); - let audible_bell_count = self.audible_bell_count; - let visual_bell_count = self.visual_bell_count; - let errors = self.errors; *self = Self::new(self.grid.size(), self.grid.scrollback_len()); self.title = title; self.icon_name = icon_name; - self.audible_bell_count = audible_bell_count; - self.visual_bell_count = visual_bell_count; - self.errors = errors; - } - - // ESC g - fn vb(&mut self) { - self.visual_bell_count += 1; } // csi codes @@ -1552,14 +1470,13 @@ impl Screen { impl vte::Perform for Screen { fn print(&mut self, c: char) { if c == '\u{fffd}' || ('\u{80}'..'\u{a0}').contains(&c) { - self.errors = self.errors.saturating_add(1); + log::debug!("unhandled text character: {c}"); } self.text(c); } fn execute(&mut self, b: u8) { match b { - 7 => self.bel(), 8 => self.bs(), 9 => self.tab(), 10 => self.lf(), @@ -1568,9 +1485,8 @@ impl vte::Perform for Screen { 13 => self.cr(), // we don't implement shift in/out alternate character sets, but // it shouldn't count as an "error" - 14 | 15 => {} + 7 | 14 | 15 => {} _ => { - self.errors = self.errors.saturating_add(1); log::debug!("unhandled control character: {b}"); } } @@ -1585,7 +1501,7 @@ impl vte::Perform for Screen { b'>' => self.deckpnm(), b'M' => self.ri(), b'c' => self.ris(), - b'g' => self.vb(), + b'g' => {} _ => { log::debug!("unhandled escape code: ESC {b}"); } diff --git a/src/state.rs b/src/state.rs new file mode 100644 index 0000000..c84fa2b --- /dev/null +++ b/src/state.rs @@ -0,0 +1,61 @@ +pub struct State<'a, T: crate::callbacks::Callbacks> { + screen: &'a mut crate::Screen, + callbacks: &'a mut T, +} + +impl<'a, T: crate::callbacks::Callbacks> State<'a, T> { + pub fn new(screen: &'a mut crate::Screen, callbacks: &'a mut T) -> Self { + Self { screen, callbacks } + } +} + +impl<'a, T: crate::callbacks::Callbacks> vte::Perform for State<'a, T> { + fn print(&mut self, c: char) { + if c == '\u{fffd}' || ('\u{80}'..'\u{a0}').contains(&c) { + self.callbacks.error(self.screen); + } + self.screen.print(c); + } + + fn execute(&mut self, b: u8) { + match b { + 7 => self.callbacks.audible_bell(self.screen), + 8..=15 => {} + _ => { + self.callbacks.error(self.screen); + } + } + self.screen.execute(b); + } + + fn esc_dispatch(&mut self, intermediates: &[u8], ignore: bool, b: u8) { + if intermediates.is_empty() && b == b'g' { + self.callbacks.visual_bell(self.screen); + } + self.screen.esc_dispatch(intermediates, ignore, b); + } + + fn csi_dispatch( + &mut self, + params: &vte::Params, + intermediates: &[u8], + ignore: bool, + c: char, + ) { + self.screen.csi_dispatch(params, intermediates, ignore, c); + } + + fn osc_dispatch(&mut self, params: &[&[u8]], bel_terminated: bool) { + self.screen.osc_dispatch(params, bel_terminated); + } + + fn hook( + &mut self, + params: &vte::Params, + intermediates: &[u8], + ignore: bool, + action: char, + ) { + self.screen.hook(params, intermediates, ignore, action); + } +} diff --git a/src/term.rs b/src/term.rs index ffa3ecb..99ca266 100644 --- a/src/term.rs +++ b/src/term.rs @@ -376,26 +376,6 @@ impl BufWrite for MoveFromTo { } } -#[derive(Default, Debug)] -#[must_use = "this struct does nothing unless you call write_buf"] -pub struct AudibleBell; - -impl BufWrite for AudibleBell { - fn write_buf(&self, buf: &mut Vec) { - buf.push(b'\x07'); - } -} - -#[derive(Default, Debug)] -#[must_use = "this struct does nothing unless you call write_buf"] -pub struct VisualBell; - -impl BufWrite for VisualBell { - fn write_buf(&self, buf: &mut Vec) { - buf.extend_from_slice(b"\x1bg"); - } -} - #[must_use = "this struct does nothing unless you call write_buf"] pub struct ChangeTitle<'a> { icon_name: &'a str, diff --git a/tests/control.rs b/tests/control.rs index 60a6320..cd9cdeb 100644 --- a/tests/control.rs +++ b/tests/control.rs @@ -2,39 +2,44 @@ mod helpers; #[test] fn bel() { + struct State { + bel: usize, + } + + impl vt100::Callbacks for State { + fn audible_bell(&mut self, _: &mut vt100::Screen) { + self.bel += 1; + } + } + let mut parser = vt100::Parser::default(); - assert_eq!(parser.screen().audible_bell_count(), 0); + let mut state = State { bel: 0 }; + assert_eq!(state.bel, 0); let screen = parser.screen().clone(); - parser.process(b"\x07"); - assert_eq!(parser.screen().audible_bell_count(), 1); - assert_eq!(parser.screen().audible_bell_count(), 1); + parser.process_cb(b"\x07", &mut state); + assert_eq!(state.bel, 1); assert_eq!(parser.screen().contents_diff(&screen), b""); - assert_eq!(parser.screen().bells_diff(&screen), b"\x07"); let screen = parser.screen().clone(); - parser.process(b"\x07"); - assert_eq!(parser.screen().audible_bell_count(), 2); + parser.process_cb(b"\x07", &mut state); + assert_eq!(state.bel, 2); assert_eq!(parser.screen().contents_diff(&screen), b""); - assert_eq!(parser.screen().bells_diff(&screen), b"\x07"); let screen = parser.screen().clone(); - parser.process(b"\x07\x07\x07"); - assert_eq!(parser.screen().audible_bell_count(), 5); + parser.process_cb(b"\x07\x07\x07", &mut state); + assert_eq!(state.bel, 5); assert_eq!(parser.screen().contents_diff(&screen), b""); - assert_eq!(parser.screen().bells_diff(&screen), b"\x07"); let screen = parser.screen().clone(); - parser.process(b"foo"); - assert_eq!(parser.screen().audible_bell_count(), 5); + parser.process_cb(b"foo", &mut state); + assert_eq!(state.bel, 5); assert_eq!(parser.screen().contents_diff(&screen), b"foo"); - assert_eq!(parser.screen().bells_diff(&screen), b""); let screen = parser.screen().clone(); - parser.process(b"ba\x07r"); - assert_eq!(parser.screen().audible_bell_count(), 6); + parser.process_cb(b"ba\x07r", &mut state); + assert_eq!(state.bel, 6); assert_eq!(parser.screen().contents_diff(&screen), b"bar"); - assert_eq!(parser.screen().bells_diff(&screen), b"\x07"); } #[test] diff --git a/tests/escape.rs b/tests/escape.rs index 478b30b..fbe7bd3 100644 --- a/tests/escape.rs +++ b/tests/escape.rs @@ -17,39 +17,44 @@ fn ris() { #[test] fn vb() { + struct State { + vb: usize, + } + + impl vt100::Callbacks for State { + fn visual_bell(&mut self, _: &mut vt100::Screen) { + self.vb += 1; + } + } + let mut parser = vt100::Parser::default(); - assert_eq!(parser.screen().visual_bell_count(), 0); + let mut state = State { vb: 0 }; + assert_eq!(state.vb, 0); let screen = parser.screen().clone(); - parser.process(b"\x1bg"); - assert_eq!(parser.screen().visual_bell_count(), 1); - assert_eq!(parser.screen().visual_bell_count(), 1); + parser.process_cb(b"\x1bg", &mut state); + assert_eq!(state.vb, 1); assert_eq!(parser.screen().contents_diff(&screen), b""); - assert_eq!(parser.screen().bells_diff(&screen), b"\x1bg"); let screen = parser.screen().clone(); - parser.process(b"\x1bg"); - assert_eq!(parser.screen().visual_bell_count(), 2); + parser.process_cb(b"\x1bg", &mut state); + assert_eq!(state.vb, 2); assert_eq!(parser.screen().contents_diff(&screen), b""); - assert_eq!(parser.screen().bells_diff(&screen), b"\x1bg"); let screen = parser.screen().clone(); - parser.process(b"\x1bg\x1bg\x1bg"); - assert_eq!(parser.screen().visual_bell_count(), 5); + parser.process_cb(b"\x1bg\x1bg\x1bg", &mut state); + assert_eq!(state.vb, 5); assert_eq!(parser.screen().contents_diff(&screen), b""); - assert_eq!(parser.screen().bells_diff(&screen), b"\x1bg"); let screen = parser.screen().clone(); - parser.process(b"foo"); - assert_eq!(parser.screen().visual_bell_count(), 5); + parser.process_cb(b"foo", &mut state); + assert_eq!(state.vb, 5); assert_eq!(parser.screen().contents_diff(&screen), b"foo"); - assert_eq!(parser.screen().bells_diff(&screen), b""); let screen = parser.screen().clone(); - parser.process(b"ba\x1bgr"); - assert_eq!(parser.screen().visual_bell_count(), 6); + parser.process_cb(b"ba\x1bgr", &mut state); + assert_eq!(state.vb, 6); assert_eq!(parser.screen().contents_diff(&screen), b"bar"); - assert_eq!(parser.screen().bells_diff(&screen), b"\x1bg"); } #[test] diff --git a/tests/helpers/mod.rs b/tests/helpers/mod.rs index 67bb1f4..963e57e 100644 --- a/tests/helpers/mod.rs +++ b/tests/helpers/mod.rs @@ -103,15 +103,6 @@ pub fn compare_screens( is!(got.title(), expected.title()); is!(got.icon_name(), expected.icon_name()); - is!( - got.audible_bell_count() > 0, - expected.audible_bell_count() > 0 - ); - is!( - got.visual_bell_count() > 0, - expected.visual_bell_count() > 0 - ); - is!(got.application_keypad(), expected.application_keypad()); is!(got.application_cursor(), expected.application_cursor()); is!(got.hide_cursor(), expected.hide_cursor()); @@ -138,13 +129,10 @@ pub fn rows_formatted_reproduces_state(input: &[u8]) -> bool { } pub fn contents_formatted_reproduces_screen(screen: &vt100::Screen) -> bool { - let empty_screen = vt100::Parser::default().screen().clone(); - let mut new_input = screen.contents_formatted(); new_input.extend(screen.input_mode_formatted()); new_input.extend(screen.title_formatted()); assert_eq!(new_input, screen.state_formatted()); - new_input.extend(screen.bells_diff(&empty_screen)); let mut new_parser = vt100::Parser::default(); new_parser.process(&new_input); let got_screen = new_parser.screen().clone(); @@ -153,8 +141,6 @@ pub fn contents_formatted_reproduces_screen(screen: &vt100::Screen) -> bool { } pub fn rows_formatted_reproduces_screen(screen: &vt100::Screen) -> bool { - let empty_screen = vt100::Parser::default().screen().clone(); - let mut new_input = vec![]; let mut wrapped = false; for (idx, row) in screen.rows_formatted(0, 80).enumerate() { @@ -170,7 +156,6 @@ pub fn rows_formatted_reproduces_screen(screen: &vt100::Screen) -> bool { new_input.extend(screen.attributes_formatted()); new_input.extend(screen.input_mode_formatted()); new_input.extend(screen.title_formatted()); - new_input.extend(screen.bells_diff(&empty_screen)); let mut new_parser = vt100::Parser::default(); new_parser.process(&new_input); let got_screen = new_parser.screen().clone(); @@ -210,14 +195,11 @@ pub fn contents_diff_reproduces_state_from_screens( let mut diff_input = screen.contents_diff(prev_screen); diff_input.extend(screen.input_mode_diff(prev_screen)); diff_input.extend(screen.title_diff(prev_screen)); - diff_input.extend(screen.bells_diff(prev_screen)); assert_eq!(diff_input, screen.state_diff(prev_screen)); let mut diff_prev_input = prev_screen.contents_formatted(); diff_prev_input.extend(screen.input_mode_formatted()); diff_prev_input.extend(screen.title_formatted()); - diff_prev_input - .extend(screen.bells_diff(vt100::Parser::default().screen())); let mut new_parser = vt100::Parser::default(); new_parser.process(&diff_prev_input); diff --git a/tests/init.rs b/tests/init.rs index 94f4403..ba54fd4 100644 --- a/tests/init.rs +++ b/tests/init.rs @@ -22,8 +22,6 @@ fn init() { assert_eq!(parser.screen().title(), ""); assert_eq!(parser.screen().icon_name(), ""); - assert_eq!(parser.screen().audible_bell_count(), 0); - assert_eq!(parser.screen().visual_bell_count(), 0); assert!(!parser.screen().application_keypad()); assert!(!parser.screen().application_cursor()); assert!(!parser.screen().hide_cursor()); -- cgit v1.2.3-54-g00ecf