Strip OSC and DCS sequences to support e.g. OSC 8 hyperlinks over tmux. #280
Conversation
Treat OSC (e.g. OSC 8 hyperlinks) as non-printing when iterating/stripping ANSI codes.
Treat DCS as non-printing, including the DCS wrapper used by tmux passthrough. Adds a regression test for tmux-wrapped OSC 8 hyperlinks.
Ensure the complex_data test fixture is present in packaged sources.
|
Appreciate the clean commit history, but it looks like there's a fair bit of code duplication. Could that be deduplicated? (If not, why not?) |
@djc Just my editorial choice to keep things simple. Did you have something more like the following (see latest commit) in mind? It's more complex but clarifies intent. (Happy to squash if you like it, honestly I kind of do.) Either way, I think the new commit at least clearly shows why there is some subtlety due to the differences in OSC and DCS semantics/idiosyncrasies---two examples:
Let me know what you like! |
|
(Sorry about the noise I was bikeshedding a function name.) |
djc
left a comment
djc
left a comment
There was a problem hiding this comment.
I definitely like it better too -- please squash.
Is there existing stuff which could leverage the same trait?
| } | ||
|
|
||
| fn consume_dcs_end_exclusive(it: &mut Peekable<CharIndices<'_>>, start: usize) -> usize { | ||
| fn consume_end_exclusive<S: EscSequence>( |
There was a problem hiding this comment.
Maybe this could/should be a provided method on the trait?
| it.next(); | ||
| return end; | ||
| } | ||
| trait EscSequence { |
There was a problem hiding this comment.
Nit: prefer top-down ordering. That means EscSequence comes after impls for it but before EscAction.
Also, within the trait I'd prefer on_escape(), on_char(), START (renamed from START_CHAR which just duplicates its type).
Maybe EscAction could be reformulated as a ControlFlow<(), bool>?
2 participants
I noticed that using
indicatif(and friends) to embed a hyperlink in a console progress bar caused the wheels to fall off my app (with many newlines being spammed to the console and the auto-updating line-rewriting broken). I tracked the problem down to the OSC sequences incorrectly being interpreted as printable, and the following patch fixes the problem for me.