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 lineHighlight for ansi theme #1730

Closed
jsit opened this issue Jul 14, 2021 · 9 comments · Fixed by #1985
Closed

Add lineHighlight for ansi theme #1730

jsit opened this issue Jul 14, 2021 · 9 comments · Fixed by #1985
Labels
feature-request New feature or request good first issue Good for newcomers themes

Comments

@jsit
Copy link

jsit commented Jul 14, 2021

The ansi theme doesn't have lineHighlight defined, so the highlighted line is not apparent.

@jsit jsit added the feature-request New feature or request label Jul 14, 2021
@sharkdp sharkdp added good first issue Good for newcomers themes labels Jul 25, 2021
@sharkdp
Copy link
Owner

sharkdp commented Jul 25, 2021

Thank you for reporting this! Should be easy to fix, I hope.

@jsit
Copy link
Author

jsit commented Jul 25, 2021

Well what I realized is that no single color will work well, depending on whether a user has a light or dark background. I imagine this is part of why there were once separate ansi-dark and ansi-light themes.

@sharkdp
Copy link
Owner

sharkdp commented Jul 25, 2021

FYI @mk12: again, only if you are interested(!).

@mk12
Copy link
Contributor

mk12 commented Jul 26, 2021

Can you share an example of when bat would use this? I'm not sure when a line would be highlighted.

@sharkdp
Copy link
Owner

sharkdp commented Jul 26, 2021

Ah, yes. Sorry. Whenever someone uses the --highlight-line option:

    -H, --highlight-line <N:M>...        Highlight lines N through M.

For example:

seq 10 | bat --highlight-line 3:5

image

@mk12
Copy link
Contributor

mk12 commented Jul 26, 2021

Thanks for the example, I didn't know about that.

Even if ansi-light and ansi-dark were still separate I don't think there's any good color for this in a typical 8 color palette. For example if you choose yellow then foreground yellow text becomes invisible. You could choose from the 256-color palette but that defeats the purpose of ansi being restricted to the 8 color palette.

I'd suggest using bold (already supported I believe) or reverse video (would need to use another alpha channel pattern to encode it). What do you think?

@jsit
Copy link
Author

jsit commented Jul 26, 2021

What's the support for underline like? I imagine it's less common in terminal emulator than bold.

@sharkdp
Copy link
Owner

sharkdp commented Jul 29, 2021

What's the support for underline like? I imagine it's less common in terminal emulator than bold.

Yes, definitely.

Let's try this out.

@mdibaiee
Copy link
Contributor

mdibaiee commented Dec 23, 2021

Hey 👋
I just made an attempt to fix this, but I'm not sure if what I've done is the best way to do it... (I have an ugly hardcoded condition for ANSI...)

Another way to do it would be to use a custom theme scope, e.g.:

<dict>
    <key>name</key>
    <string>Line highlight style</string>
    <key>scope</key>
    <string>line_highlight</string>
    <key>settings</key>
    <dict>
        <key>font_style</key>
        <string>italic</string>
    </dict>
</dict>

What do you think?

Update: I implemented the custom scope solution and committed it on top of my MR. There are two commits now each with one of the approaches. Let me know what you think and I can squash to the final solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request good first issue Good for newcomers themes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants