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

feat: support bg color in code blocks #223

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Valentin271
Copy link
Contributor

@Valentin271 Valentin271 commented Jan 25, 2024

In order to fix #211, I've tried adding support for background colors.

I just can't get the final Text render to work because I'm not sure where it is.

I think if Attr had bg_color or similar here that could work. But that's a cosmic_text struct and cosmic_text doesn't mention background anywhere in their doc.

So I'm kind of lost here, I tried looking at how TextBox renders its background but couldn't find it either.

PS. I'll check if the tests changes are valid once I can get this to work.

@CosmicHorrorDev CosmicHorrorDev self-requested a review January 25, 2024 17:06
@CosmicHorrorDev
Copy link
Collaborator

I'm too busy with work to take a look today/tomorrow, but I should have time to try and get things working this weekend

The renderer is pretty foreign to me too, so I don't have much guidance without digging in myself unfortunately 😅

@Valentin271
Copy link
Contributor Author

No problem at all! Review when you have time and motivation 🙂.

In the meantime I'll still try to get this working.

@CosmicHorrorDev CosmicHorrorDev added C-bug Category: Something isn't working C-enhancement Category: New feature or request and removed C-bug Category: Something isn't working labels Jan 27, 2024
@CosmicHorrorDev
Copy link
Collaborator

CosmicHorrorDev commented Jan 27, 2024

So, this is the area where the background of the entire codeblock gets drawn, so it'll also be where the background color for the individual lines get drawn as well

https://github.com/trimental/inlyne/blob/a038ae95acb31ead8b632c30f984c05d0878bf97/src/renderer.rs#L249-L274

I also think the default background color and style changes can be dropped in text.rs since that's handling different stuff


Feel free to ask questions if things are still unclear. I'd also be happy to take a stab at getting things working if you get stuck

@Valentin271 Valentin271 force-pushed the feat/support-bg-color branch from 4c46a04 to e089856 Compare February 4, 2024 18:24
@Valentin271 Valentin271 marked this pull request as draft February 18, 2024 16:41
@Valentin271
Copy link
Contributor Author

Thanks for the help, I've got some background rendering to work. However there is a lot of bug right now and the code is absolutely hideous.

  • Doesn't work with soft wrap
  • For some reason background color of normal code is dark red (I've checked this is not on syntect end's), but the logs shows bg_color gets set to this at some point
  • The code block has a higher height the the sum of Texts, making it visible at the bottom of a diff
  • I think the colored rectangle is not very well positioned on the line, i.e. It probably need to be a tiny bit lower

Here is a screenshot of the render as of now:
image

To do this I'm actually missing some position/size helpers on Text. It there any that I've missed?
Also I'd need to iterate over lines, I wouldn't be happy with letting if text == "\n" in a production code.

@CosmicHorrorDev
Copy link
Collaborator

Thanks for all your work on this! I should have some time to look over things either today or tomorrow

@CosmicHorrorDev
Copy link
Collaborator

CosmicHorrorDev commented Feb 21, 2024

Sorry for the delay. I'm on vacation, so I probably won't have much time to look into this for the next week or so (Me thinking I would have enough spare time was a bit optimistic 😅)

@Valentin271
Copy link
Contributor Author

Hey, no problem, have a great vacation! I'm still working on this when I have time (essentially week-ends). I think I need to refactor some of the code around though to make this specific change easier (to do and review).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

regression: no more diff coloration
2 participants