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

Jump to bottom

tmux-cc: force DcsPassThrough in tmux-cc mode #977

Closed
wants to merge 1 commit into from
Closed

tmux-cc: force DcsPassThrough in tmux-cc mode #977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 11 additions & 1 deletion termwiz/src/escape/parser/mod.rs
View file
Open in desktop
Expand Up @@ -23,6 +23,7 @@ struct SixelBuilder {
struct ParseState {
sixel: Option<SixelBuilder>,
dcs: Option<ShortDeviceControl>,
tmux_cc: bool
}

/// The `Parser` struct holds the state machine that is used to decode
Expand Down Expand Up @@ -55,7 +56,13 @@ impl Parser {
callback: &mut callback,
state: &mut self.state.borrow_mut(),
};
self.state_machine.parse(bytes, &mut perform);

if perform.state.tmux_cc {
self.state_machine.force_dcs_passthrough(bytes, &mut perform);
Copy link
Owner

Choose a reason for hiding this comment

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

I haven't looked to make sure this suggestion is good, but I think that we could add DeviceControlMode::Bytes(Vec<u8>) and pass that bytes through at the termwiz layer rather than having this special state cut across to the vtparse layer.

For that to work, I think we need a bit more state for tmux in here, so that we can match for a sequence like \n%exit to break out of tmux-cc mode.

When in tmux-cc mode, we can simply bypass the vtparse parser and redirect straight through to DeviceControlMode::Bytes which would get handled in the same way as DeviceControlMode::Data, but with the advantage that it's a bit more efficient to pass the vec through there, rather than the individual Data items.

}
else {
self.state_machine.parse(bytes, &mut perform);
}
}

/// A specialized version of the parser that halts after recognizing the
Expand Down Expand Up @@ -173,6 +180,9 @@ impl<'a, F: FnMut(Action)> VTActor for Performer<'a, F> {
data: vec![],
});
} else {
if byte == b'p' && params == vec![1000] {
Copy link
Owner

Choose a reason for hiding this comment

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

I think we can avoid making a vec for this; the vec is undesirable because it represents an unconditional heap allocation just for a comparison.

Suggested change
if byte == b'p' && params == vec![1000] {
if byte == b'p' && params == [1000] {

self.state.tmux_cc = true;
Copy link
Owner

Choose a reason for hiding this comment

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

I don't see anything that will exit this state, so as-written, this feels like it will "break" the pane when the mode is exited :-/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think in iTerm2 you need to manually break the state. In Wezterm I guess "press q"? I'll see where to add this.

}
(self.callback)(Action::DeviceControl(DeviceControlMode::Enter(Box::new(
EnterDeviceControlMode {
byte,
Expand Down
8 changes: 8 additions & 0 deletions vtparse/src/lib.rs
View file
Open in desktop
Expand Up @@ -679,6 +679,14 @@ impl VTParser {
self.parse_byte(*b, actor);
}
}

/// Force DCS pass through for tmux cc
pub fn force_dcs_passthrough(&mut self, bytes: &[u8], actor: &mut dyn VTActor) {
for b in bytes {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a way to batch pass through the entire buffer? That would help the performance a lot.

Copy link
Owner

Choose a reason for hiding this comment

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

Ah, this bit is in vtparse. Layering wise this makes things very difficult. I'd suggest doing this rerouting in the termwiz crate where we have the higher level state instead of here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood. I'll try to think of something else. By the way I have an experimental branch tmux-cc at https://github.com/skyline75489/wezterm/tree/dev/tmux-cc-prototype, which handles the output part, very roughly but works. Take a look if you want :)

(bear with all my amateur Rust code)

let action = Action::Put;
self.action(action, *b, actor);
}
}
}

#[cfg(test)]
Expand Down