regression: no more diff coloration

[Valentin271]

In inlyne 0.3.3, ```diff were rendered as follow (the whole line colored).

With inlyne main, ```diff renders as follow (only + colored).

---

After more research, it looks like commit 04ef6e1 introduced this behavior.

I can probably take a look at that but not in the immediate future.

[CosmicHorrorDev]

Just guessing from where things regressed, but it's probably just that the new default light theme doesn't support highlighting diffs correctly (which isn't surprising because the themes embedded with syntect are of mixed maintenance status)

We have access to a lot more themes through two-face now (thanks to bat), so it would probably be good to evaluate and switch over to (potentially a subset of) those themes

[Valentin271]

Good to know, i'll take a look at it when possible.

I thought of the theme support too but the colored + threw me off. I'll check and see.

[Valentin271]

I did a little bit of research on that today, I can confirm this simply comes from InspiredGithub theme not supporting diff correctly.

I think we should switch to using two-faces theme so people have more choice.

Do you remember why you switched to InspiredGithub? I think default dark and light color_highlighter should be variants of the same theme (e.g. Base16Ocean{Dark,Light}, OneHalf{Dark,Light}).

[CosmicHorrorDev]

We originally switched away from a light/dark variant theme due to the light theme having very low-contrast colors (Base16Mocha IIRC)

I picked inspiredgithub since it seemed to have much better contrast, but that was before we added two-face. I've got some free time today, so I can work on integrating two-face's themes

[Valentin271]

I thought Base16OceanLight had low contrast too.
I tried some themes today, I think monokai and OneHalf (but diff colors are too muted on this one imo) are the best options (that have variants, support diff and have decent colors overall).

Here is a quick comparison:

[CosmicHorrorDev]

#219 exposes a lot more themes if you want to take a look at those too

Also worth noting that it looks like a lot of themes mess up that diff highlighting, but we can always patch individual themes to fix little issues like that (depending on how hard that ends up being)

[Valentin271]

I already tried those by changing the code directly (see my comparing screenshot), tried again and monokai is definitely the best imo.

we can always patch individual themes

We could also fix this upstream, currently it's an issue in bat, two-face and inlyne at the very least.
I can probably take a look at this.

[CosmicHorrorDev]

I already tried those by changing the code directly (see my comparing screenshot), tried again and monokai is definitely the best imo.

Ah apologies, that does make more sense now. I agree that monokai is likely the best (although I do really like Coldark-Cold/Coldark-Dark too)

We could also fix this upstream, currently it's an issue in bat, two-face and inlyne at the very least.
I can probably take a look at this.

I agree, patching upstream is the path of least resistance in the long run. bat mostly just aggregates different themes from different repos (themes within https://github.com/sharkdp/bat/tree/master/assets) along with having the ability to patch individual files (the patches dir in that same link). I'm not sure what the criteria is on when patching is okay vs when to fix upstream and then update the submodule in bat

Also worth noting that I'd like to start adding tests for individual themes+syntax highlighting within two-face itself so that we can prevent regressions in the future without having to test everything downstream here

[Valentin271]

although I do really like Coldark-Cold/Coldark-Dark too

True, they do look good too. The darker background on light theme helps a lot. Let's go for that.

Also worth noting that I'd like to start adding tests for individual themes+syntax highlighting within two-face itself so that we can prevent regressions in the future without having to test everything downstream here

Yeah that would be a good idea. Just looking at the example you provided in the readme, looks like this could be easily tested with insta.

[Valentin271]

So I've played around with the values of the github theme, there is actually nothing to patch as the style is present (e.g. markup.deleted) and even works perfectly.

Long story short I ended up using a color pick on the letters of a diff and ... yeah it's the right color, it's just so close to black it's hard to see.

The actual problem is that the github theme uses background color for diffs, and with them it would be way easier to see the colors.
I don't know how hard it would be to support background colors for code block, probably somewhat internal to syntect.

[CosmicHorrorDev]

(The good news is that means a lot of the foundational work is already supported. I can elaborate more after I get off of work)

[CosmicHorrorDev]

Alright, so the good news is that the background color for the individual lines is already included in the HTML that comrak gives us which means that both synctect and comrak are doing their individual things correctly

```diff
+ Ins
- Del
```

gives us

<pre style="background-color:#ffffff;"><code class="language-diff"><span style="background-color:#ddffdd;color:#003300;">+ Ins
</span><span style="background-color:#ffdddd;color:#770000;">- Del
</span></code></pre>

and we already support parsing that, so we just need to add an arm for Style::BackgroundColor here

https://github.com/trimental/inlyne/blob/5bca55d504d57d7f1eba8cfecbc096ca3bfc0a80/src/interpreter/mod.rs#L489-L497

That gets all of the easy stuff out of the way which finally leaves the hard parts of

1. Storing that value with the individual crate::text::Text snippets and finally
2. Actually rendering the background color of crate::text::Text's individually since it's currently for entire crate::text::TextBoxes instead

[CosmicHorrorDev]

I've gone ahead and merged #219, so we can handle switching the default dark/light syntax highlighting theme if you think there's still enough reason to do so (beyond the bug with inlyne not supporting background color on spans)

[Valentin271]

If we can fix the background color in code blocks and the diff/colors looks ok, then there should be no reason to change the default theme.

I've quickly looked at supporting that, I think I can do it I just couldn't really find where the background color was actually drawn.