Skip to content

Support for dim formatting#9

Closed
dfaust wants to merge 1 commit intodoy:mainfrom
dfaust:master
Closed

Support for dim formatting#9
dfaust wants to merge 1 commit intodoy:mainfrom
dfaust:master

Conversation

@dfaust
Copy link
Copy Markdown
Contributor

@dfaust dfaust commented Mar 22, 2023

Uses the code 2 to initiate dim formatting and 22 to exit it. 22 is also used to exit bold formatting.

I looked the codes up at https://en.wikipedia.org/wiki/ANSI_escape_code#SGR_(Select_Graphic_Rendition)_parameters

Copy link
Copy Markdown
Owner

@doy doy left a comment

Choose a reason for hiding this comment

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

this overall looks reasonable, other than the thing i noted. could you also add some tests?

Comment thread src/term.rs
if dim {
write_param!(2);
} else {
write_param!(22);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this isn't correct - text that is only bold will end up no longer being bold after this, because turning off dim also turns off bold. you'll probably need to handle these cases together to make sure all of the different potential settings work together properly.

chris-olszewski added a commit to vercel/turborepo that referenced this pull request Mar 6, 2024
### Description

Due to our use case we need to extend `vt100` with some functionality.
The functionality can be added upstream, but based on the age of PRs on
the project, we cannot wait for this to happen.

Reviewing instructions:
I quick documented changes I made from the straight `vt100` crate.
Significant behavior/bug fixes will be added in future PRs

General plans of changes:
- Cherry pick [underflow fix](doy/vt100-rust#11)
 - [Support dimmed formatting](doy/vt100-rust#9)
- Add a version of `Screen` that displays *all* content including lines
that are now in the scrollback
 - Some [perf fixes](doy/vt100-rust#14)

### Testing Instructions

Verified existing test suite passes on my machine. Verify test suite
passes on CI
@doy
Copy link
Copy Markdown
Owner

doy commented Jan 11, 2025

merged, thanks!

@doy doy closed this Jan 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants