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.

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

Add CSI subparameter support #65

Merged
merged 4 commits into from
Aug 5, 2020

Conversation

chrisduerr
Copy link
Member

This adds support for CSI subparameters like \x1b[38:2:255:0:255m,
which allows the combination of truecolor SGR commands together with
other SGR parameters like bold text, without any ambiguity.

This implements subparameters by storing them in a list together with
all other parameters and having a separate slice to indicate which
parameter is a subparameter and how long the subparameter list is. This
allows for static memory allocation and good performance while still
having the option for dynamic sizing of the parameters. Since the
subparameters are now also counted as parameters, the number of allowed
parameters has been increased from 16 to 32.

Since the existing structures combine the handling of parameters for CSI
and DCS escape sequences, it is now also possible for DCS parameters to
have subparameters, even though that is currently never used.
Considering that DCS is rarely supported by terminal emulators, handling
these separately would likely just cause unnecessary issues. The
performance should also be better by using this existing subparam
structure rather than having two separate structures for DCS and CSI
parameters.

The only API provided for accessing the list of parameters is using an
iterator, this is intentional to make the internal structure clear and
allow for easy optimizations downstream. Since it makes little sense to
access parameters out of order, this limitation should not have any
negative effects on performance. The main drawback is that direct access
to the first parameter while ignoring all other subparameters is less
efficient, since it requires indexing a slice after iterating to the
element. However while this is often useful, it's mostly done for the
first few parameters which significantly reduces the overhead to a
negligible amount. At the same time this forces people to support
subparameters or at least consider their existence, which should make it
more difficult to implement things improperly downstream.

Fixes #22.

This adds support for CSI subparameters like `\x1b[38:2:255:0:255m`,
which allows the combination of truecolor SGR commands together with
other SGR parameters like bold text, without any ambiguity.

This implements subparameters by storing them in a list together with
all other parameters and having a separate slice to indicate which
parameter is a subparameter and how long the subparameter list is. This
allows for static memory allocation and good performance while still
having the option for dynamic sizing of the parameters. Since the
subparameters are now also counted as parameters, the number of allowed
parameters has been increased from `16` to `32`.

Since the existing structures combine the handling of parameters for CSI
and DCS escape sequences, it is now also possible for DCS parameters to
have subparameters, even though that is currently never used.
Considering that DCS is rarely supported by terminal emulators, handling
these separately would likely just cause unnecessary issues. The
performance should also be better by using this existing subparam
structure rather than having two separate structures for DCS and CSI
parameters.

The only API provided for accessing the list of parameters is using an
iterator, this is intentional to make the internal structure clear and
allow for easy optimizations downstream. Since it makes little sense to
access parameters out of order, this limitation should not have any
negative effects on performance. The main drawback is that direct access
to the first parameter while ignoring all other subparameters is less
efficient, since it requires indexing a slice after iterating to the
element. However while this is often useful, it's mostly done for the
first few parameters which significantly reduces the overhead to a
negligible amount. At the same time this forces people to support
subparameters or at least consider their existence, which should make it
more difficult to implement things improperly downstream.

Fixes alacritty#22.
Copy link
Member Author

@chrisduerr chrisduerr left a comment

Choose a reason for hiding this comment

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

@kchibisov This implementation should make subparameters work in Alacritty, without any significant performance impact. I'm curious if you have any general concerns with this approach, otherwise I'd go ahead and implement this in Alacritty to assure there really is no negative performance impact.

If both Alacritty and vte versions are implemented without any measurable performance hit, we can move ahead and review/merge this.

src/table.rs Outdated
@@ -139,7 +136,7 @@ generate_state_changes!(state_changes, {
0x19 => (Anywhere, Ignore),
0x1c..=0x1f => (Anywhere, Ignore),
0x30..=0x39 => (Anywhere, Param),
0x3b => (Anywhere, Param),
0x3a..=0x3b => (Anywhere, Param),
0x7f => (Anywhere, Ignore),
0x3a => (DcsIgnore, None),
Copy link
Member

Choose a reason for hiding this comment

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

I bet you should remove this, since you've merged it above.

src/params.rs Show resolved Hide resolved
@chrisduerr chrisduerr merged commit 4f44023 into alacritty:master Aug 5, 2020
@chrisduerr chrisduerr deleted the colon_separation branch August 5, 2020 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Implement colon separated CSI parameters
2 participants