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.

Sign up for GitHub

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

Added APC support #110

Closed
wants to merge 10 commits into from
Closed

Added APC support #110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1af8b6e
Adds support for APC (Application Program Commands) within the applic…
John-Toohey Jan 21, 2024
955d293
Rewrote APC to be more similar to OSC
John-Toohey Jan 21, 2024
b6b8e50
Merged the OSC and APC buffers
John-Toohey Jan 21, 2024
946346a
cargo fmt
John-Toohey Jan 21, 2024
e3efc4e
Added support for APC / SOS / PM messages
John-Toohey Feb 25, 2024
cc99a0f
Removed eprintln! statements for debugging
John-Toohey Feb 25, 2024
e2c61e2
Merge branch 'apc' into apc2
John-Toohey Mar 9, 2024
642e207
cargo fmt
John-Toohey Mar 9, 2024
8e10a22
Added some fullstops to comments
John-Toohey Mar 9, 2024
bb85177
Merge branch 'master' into apc
chrisduerr Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/definitions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub enum State {
#[default]
Ground = 12,
OscString = 13,
SosPmApcString = 14,
Null = 14,
Copy link
Member

Choose a reason for hiding this comment

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

This is just a bad name. A "null state" already exists, it's called Ground. Looking at State::Null alone would make it impossible to tell what that state is for.

Utf8 = 15,
}

Expand All @@ -35,9 +35,9 @@ pub enum Action {
Execute = 5,
Hook = 6,
Ignore = 7,
OscEnd = 8,
OscPut = 9,
OscStart = 10,
StringEnd = 8,
StringPut = 9,
StringStart = 10,
Param = 11,
Print = 12,
Put = 13,
Expand Down Expand Up @@ -76,7 +76,7 @@ mod tests {
#[test]
fn unpack_state_action() {
match unpack(0xee) {
(State::SosPmApcString, Action::Unhook) => (),
(State::Null, Action::Unhook) => (),
_ => panic!("unpack failed"),
}

Expand All @@ -94,7 +94,7 @@ mod tests {
#[test]
fn pack_state_action() {
match unpack(0xee) {
(State::SosPmApcString, Action::Unhook) => (),
(State::Null, Action::Unhook) => (),
_ => panic!("unpack failed"),
}

Expand Down
157 changes: 135 additions & 22 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ pub struct Parser<const OSC_RAW_BUF_SIZE: usize = MAX_OSC_RAW> {
osc_num_params: usize,
ignoring: bool,
utf8_parser: utf8::Parser,
str_start: u8,
}

impl Parser {
Expand Down Expand Up @@ -185,7 +186,7 @@ impl<const OSC_RAW_BUF_SIZE: usize> Parser<OSC_RAW_BUF_SIZE> {
self.perform_action(performer, Action::Unhook, byte);
},
State::OscString => {
self.perform_action(performer, Action::OscEnd, byte);
self.perform_action(performer, Action::StringEnd, byte);
},
_ => (),
}
Expand All @@ -200,7 +201,7 @@ impl<const OSC_RAW_BUF_SIZE: usize> Parser<OSC_RAW_BUF_SIZE> {
self.perform_action(performer, Action::Hook, byte);
},
State::OscString => {
self.perform_action(performer, Action::OscStart, byte);
self.perform_action(performer, Action::StringStart, byte);
},
_ => (),
}
Expand Down Expand Up @@ -231,6 +232,21 @@ impl<const OSC_RAW_BUF_SIZE: usize> Parser<OSC_RAW_BUF_SIZE> {
}
}

#[inline]
fn apc_dispatch<P: Perform>(&self, performer: &mut P) {
performer.apc_dispatch(&self.osc_raw);
}

#[inline]
fn pm_dispatch<P: Perform>(&self, performer: &mut P) {
performer.pm_dispatch(&self.osc_raw);
}

#[inline]
fn sos_dispatch<P: Perform>(&self, performer: &mut P) {
performer.sos_dispatch(&self.osc_raw);
}

#[inline]
fn perform_action<P: Perform>(&mut self, performer: &mut P, action: Action, byte: u8) {
match action {
Expand All @@ -246,11 +262,12 @@ impl<const OSC_RAW_BUF_SIZE: usize> Parser<OSC_RAW_BUF_SIZE> {
performer.hook(self.params(), self.intermediates(), self.ignoring, byte as char);
},
Action::Put => performer.put(byte),
Action::OscStart => {
Action::StringStart => {
self.str_start = byte % 64;
self.osc_raw.clear();
self.osc_num_params = 0;
},
Action::OscPut => {
Action::StringPut => {
#[cfg(feature = "no_std")]
{
if self.osc_raw.is_full() {
Expand All @@ -261,7 +278,7 @@ impl<const OSC_RAW_BUF_SIZE: usize> Parser<OSC_RAW_BUF_SIZE> {
let idx = self.osc_raw.len();

// Param separator
if byte == b';' {
if self.str_start == 0x1d && byte == b';' {
Copy link
Member

Choose a reason for hiding this comment

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

One extra branch isn't going to be the end of the world, but I don't see how this isn't just going to be a net-negative for anyone who doesn't use APC escapes (which is basically everyone).

let param_idx = self.osc_num_params;
match param_idx {
// Only process up to MAX_OSC_PARAMS
Expand All @@ -285,29 +302,45 @@ impl<const OSC_RAW_BUF_SIZE: usize> Parser<OSC_RAW_BUF_SIZE> {
self.osc_raw.push(byte);
}
},
Action::OscEnd => {
Action::StringEnd => {
let param_idx = self.osc_num_params;
let idx = self.osc_raw.len();

match param_idx {
// Finish last parameter if not already maxed
MAX_OSC_PARAMS => (),

// First param is special - 0 to current byte index
0 => {
self.osc_params[param_idx] = (0, idx);
self.osc_num_params += 1;
match self.str_start {
0x1d => {
match param_idx {
// Finish last parameter if not already maxed
MAX_OSC_PARAMS => (),

// First param is special - 0 to current byte index
0 => {
self.osc_params[param_idx] = (0, idx);
self.osc_num_params += 1;
},

// All other params depend on previous indexing
_ => {
let prev = self.osc_params[param_idx - 1];
let begin = prev.1;
self.osc_params[param_idx] = (begin, idx);
self.osc_num_params += 1;
},
}
self.osc_dispatch(performer, byte);
},

// All other params depend on previous indexing
_ => {
let prev = self.osc_params[param_idx - 1];
let begin = prev.1;
self.osc_params[param_idx] = (begin, idx);
self.osc_num_params += 1;
0x18 => {
self.sos_dispatch(performer);
},
0x1e => {
self.pm_dispatch(performer);
},
0x1F => {
self.apc_dispatch(performer);
},
// This condition should never be hit under the
// current way in which the code is ran.
_ => unreachable!(),
Comment on lines +340 to +342
Copy link
Member

Choose a reason for hiding this comment

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

This comment is basically just a long way to say unreachable!(). It doesn't explain why it is unreachable it just states that it is, which unreachable!() does too. Should either add a useful comment or remove this.

}
Comment on lines +309 to 343
Copy link
Member

Choose a reason for hiding this comment

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

This basically feels like an embedded parser. Most of our parser is structured in a way to stream through bytes and process them as they're coming in, however rather than creating state transitions you're storing a state transition to later then match on it instead. VTE is already too slow and I'm afraid this will just amplify that problem.

self.osc_dispatch(performer, byte);
},
Action::Unhook => performer.unhook(),
Action::CsiDispatch => {
Expand Down Expand Up @@ -428,6 +461,18 @@ pub trait Perform {
/// The `ignore` flag indicates that more than two intermediates arrived and
/// subsequent characters were ignored.
fn esc_dispatch(&mut self, _intermediates: &[u8], _ignore: bool, _byte: u8) {}

/// Called at the end of an APC (application program command), where
/// `bytes` is the content of the APC.
fn apc_dispatch(&mut self, _bytes: &[u8]) {}

/// Called at the end of an SOS (start of string), where
/// `bytes` is the content of the SOS.
fn sos_dispatch(&mut self, _bytes: &[u8]) {}

/// Called at the end of a PM (privacy message), where
/// `bytes` is the content of the PM.
fn pm_dispatch(&mut self, _bytes: &[u8]) {}
}

#[cfg(all(test, feature = "no_std"))]
Expand Down Expand Up @@ -460,6 +505,9 @@ mod tests {
DcsHook(Vec<Vec<u16>>, Vec<u8>, bool, char),
DcsPut(u8),
DcsUnhook,
Apc(Vec<u8>),
Pm(Vec<u8>),
Sos(Vec<u8>),
}

impl Perform for Dispatcher {
Expand Down Expand Up @@ -492,6 +540,18 @@ mod tests {
fn unhook(&mut self) {
self.dispatched.push(Sequence::DcsUnhook);
}

fn apc_dispatch(&mut self, bytes: &[u8]) {
self.dispatched.push(Sequence::Apc(bytes.to_vec()))
}

fn sos_dispatch(&mut self, bytes: &[u8]) {
self.dispatched.push(Sequence::Sos(bytes.to_vec()))
}

fn pm_dispatch(&mut self, bytes: &[u8]) {
self.dispatched.push(Sequence::Pm(bytes.to_vec()))
}
}

#[test]
Expand Down Expand Up @@ -880,6 +940,59 @@ mod tests {
assert_eq!(dispatcher.dispatched[6], Sequence::DcsUnhook);
}

#[test]
fn parse_apc() {
const INPUT: &[u8] = b"\x1b_abc\x1b\\";

// Test with ESC \ terminator.

let mut dispatcher = Dispatcher::default();
let mut parser = Parser::new();

for byte in INPUT {
parser.advance(&mut dispatcher, *byte);
}
assert_eq!(
dispatcher.dispatched,
vec![Sequence::Apc(b"abc".to_vec()), Sequence::Esc(vec![], false, 92)]
)
}
#[test]
fn parse_pm() {
const INPUT: &[u8] = b"\x1b^abc\x1b\\";

// Test with ESC \ terminator.

let mut dispatcher = Dispatcher::default();
let mut parser = Parser::new();

for byte in INPUT {
parser.advance(&mut dispatcher, *byte);
}
assert_eq!(
dispatcher.dispatched,
vec![Sequence::Pm(b"abc".to_vec()), Sequence::Esc(vec![], false, 92)]
)
}

#[test]
fn parse_sos() {
const INPUT: &[u8] = b"\x1bXabc\x1b\\";

// Test with ESC \ terminator.

let mut dispatcher = Dispatcher::default();
let mut parser = Parser::new();

for byte in INPUT {
parser.advance(&mut dispatcher, *byte);
}
assert_eq!(
dispatcher.dispatched,
vec![Sequence::Sos(b"abc".to_vec()), Sequence::Esc(vec![], false, 92)]
)
}

#[test]
fn intermediate_reset_on_dcs_exit() {
static INPUT: &[u8] = b"\x1bP=1sZZZ\x1b+\x5c";
Expand Down
16 changes: 6 additions & 10 deletions src/table.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ generate_state_changes!(state_changes, {
0x5b => (CsiEntry, None),
0x5d => (OscString, None),
0x50 => (DcsEntry, None),
0x58 => (SosPmApcString, None),
0x5e => (SosPmApcString, None),
0x5f => (SosPmApcString, None),
0x58 => (OscString, None),
0x5e => (OscString, None),
0x5f => (OscString, None),
},

EscapeIntermediate {
Expand Down Expand Up @@ -152,12 +152,8 @@ generate_state_changes!(state_changes, {
0x9c => (Ground, None),
},

SosPmApcString {
0x00..=0x17 => (Anywhere, Ignore),
0x19 => (Anywhere, Ignore),
0x1c..=0x1f => (Anywhere, Ignore),
0x20..=0x7f => (Anywhere, Ignore),
0x9c => (Ground, None),
Null {
0x00..=0xFF => (Anywhere, Ignore),
},

OscString {
Expand All @@ -166,6 +162,6 @@ generate_state_changes!(state_changes, {
0x08..=0x17 => (Anywhere, Ignore),
0x19 => (Anywhere, Ignore),
0x1c..=0x1f => (Anywhere, Ignore),
0x20..=0xff => (Anywhere, OscPut),
0x20..=0xff => (Anywhere, StringPut),
}
});