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

ansi parser: fix dim / half-bright #563

Merged
merged 2 commits into from
Nov 12, 2024
Merged

Conversation

hellux
Copy link
Contributor

@hellux hellux commented Apr 2, 2024

\x1b[2m seems to be parsed as DIM | UNDERLINE | BLINK | REVERSE instead of just DIM.

Not sure why the current implementation does this, the message for the commit that added it just mentions implementing multiple parameters, but not specifically why !BOLD was added:

commit d902d65fae1294d7dea5f3dcd90ad31b5b55d1f9
Author: Jinzhou Zhang <[email protected]>
Date:   Sat Jul 13 23:41:28 2019 +0800

    fix #194: color not working with ag

    It's because ag uses multiple attributes in SGR parameters.
    e.g. `\e[1;31m` which means set the attribute to BOLD and forground
    color to yellow.

    Previously skim's CSI parsing logic will take only the first parameter.

diff --git a/src/ansi.rs b/src/ansi.rs
index 8495d82..583d19b 100644
--- a/src/ansi.rs
+++ b/src/ansi.rs
@@ -69,60 +70,88 @@ impl Perform for ANSIParser {
         }
:
                 0 => attr = Attr::default(),
                 1 => attr.effect |= Effect::BOLD,
+                2 => attr.effect |= !Effect::BOLD,
                 4 => attr.effect |= Effect::UNDERLINE,
                 5 => attr.effect |= Effect::BLINK,
:

None of the tests added in this commit use DIM either, they still pass after my change, so I'm assuming this change was accidental.

@yazgoo
Copy link
Collaborator

yazgoo commented Nov 7, 2024

what does it look like in your terminal emulator before and after the change ?

@hellux
Copy link
Contributor Author

hellux commented Nov 7, 2024

When using just \033[2m (dim) on current master, e.g.

printf "\033[2mdim\033[0m\nnormal\nselected" | cargo run --release -- --ansi

it becomes dim, underline and reverse:

before

After this change becomes just dim:

after

@LoricAndre LoricAndre added the tui label Nov 7, 2024
@LoricAndre LoricAndre merged commit 17c38c5 into skim-rs:master Nov 12, 2024
5 checks passed
@LoricAndre LoricAndre added this to the v0.11.0 milestone Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants