-
Notifications
You must be signed in to change notification settings - Fork 7
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
Optionally produce css classes instead of inline styles? #65
Comments
Yes, that would be useful. Right now the code that generates the HTML is all C, so supporting arbitrary class strings will be annoying (not sure how annoying, haven't looked at the code in ages). If it's just the 8 we can have a mode that does a string replacement of the inlined styles with classes (literally gsub the hardcoded values for class names), all from R-level. That's going to slow things down a little but, but should be fine for just the 8. For 256 it becomes a little harder. Presumably this is only of interest for colors. What about text-styling (underline, bold, etc.). Those okay inline? I imagine they would be. If you have a time frame that would make this useful / less useful to you let me know. Absent some dates I'm unlikely to get to this in the near future. I'm not actively developing the package (new features) so usually I make changes if time allows when R breaks something that requires a CRAN re-submission. |
Yeah, just the eight would be fine I think. No rush from my end; when it happens I'll incorporate it into downlit but there's no urgent need for it. |
Note to self: evaluate consequences of |
In case you're interested, there's a bit more motivation at https://www.tidyverse.org/blog/2020/06/dplyr-1-0-0/#a-small-teaser. The challenge of inline styles is that they have precedence over classes, which makes the output currently hard to integrate with other style sheets. |
Agree this completely makes sense. I would encourage you to give me some "this would be useful to me prior to yyyy-mm-dd" date otherwise I'm going to keep kicking the can down the road possibly for a long time while catching up on other stuff I'm behind on (and whatever other bright shiny thing catches my attention at the moment). Right now the earliest I would envision getting to this is 1 month, optimistically, and again that won't happen without an explicit prod to do so. |
I don't think I have time to do this ahead of my next submission, but @hadley it would be useful to confirm exactly what you are hoping for. The original Rstudio issue references 16 colors (0-15). These may be specified AFAICT either via the basic ANSI CSI SGR (e.g "30-37" + "90-97", and background equivalents), or by the 256 color mode (e.g. "38;5;0-15" + "48;5;0-15"). So it's a matter of how Rstudio is encoding the colors with SGR. |
I think the goal would be a mode than translates the 16 basic colours in to css classes and 256 colour mode to inline styles. |
How sure are you that Rstudio (or whatever the source of the CSI SGR you're dealing with) is not implementing the 16 basic colors as the first 16 of the 256 color mode? I'm apologize if I'm telling you stuff you already know, but it's possible to specify the 16 basic colors in two ways (wiki page, compare 3-4 bit vs 8 bit color modes):
My plan right now is to only map 1. to classes. I'm hesitant to do 2. because that means different treatment for the first 16 colors vs the remaining 240 when in 256 color mode. The reason I ask is that looking at the details behind the issue you link and actually clicking through to the files in question, I see classes defined for all 256 colors, not just the first 16. This strongly suggests to me that the encoding you're dealing with is using the "38;5;xxx" form, not the basic form. |
@gaborcsardi/@jimhester can you comment here? The goal is to give downlit/pkgdown (via cli & fansi) the ability to produce css classes for the 16 basic ansi colours so that they can be styled in coordination with the rest of the syntax highlighting theme. |
FWIW I just started to implement the 256 color version (or optionally 8 or 16, i.e. I changed my plan and I'm allowing for both 1. and 2. and more). So so long as you're not getting true color codes (i.e. 38;2;x;x;x;) it should be okay. |
I think what @brodieG is doing will work for our use case. cc @gaborcsardi, it looks like Hadley's CC above did not work right. |
This is implemented and on CRAN. R/Ts to help promote the new feature are appreciated. |
# fansi Release Notes ## v0.5.0 * [#65](brodieG/fansi#65): `sgr_to_html` optionally converts CSI SGR to classes instead of inline styles (h/t @hadley). * [#69](brodieG/fansi#69): `sgr_to_html` is more disciplined about emitting unnecessary HTML (h/t @hadley). * New functions: * `sgr_256`: Display all 256 8-bit colors. * `in_html`: Easily output HTML in a web page. * `make_styles`: Easily produce CSS that matches 8-bit colors. * Adjust for changes to `nchar(..., type='width')` for C0-C1 control characters in R 4.1. * Restore tests bypassed in 0.4.2. ## v0.4.2 * Temporarily bypass tests due to R bug introduced in R-devel 79799. ## v0.4.1 * Correctly define/declare global symbols as per WRE 1.6.4.1, (h/t Professor Ripley, Joshua Ulrich for example fixes). * [#59](brodieG/fansi#59): Provide a `split.nl` option to `set_knit_hooks` to mitigate white space issues when using blackfriday for the markdown->html conversion (@krlmlr).
This would be useful because (e.g.) RStudio themes use classes so that you can style the numbered colours: rstudio/rstudio#1959
The text was updated successfully, but these errors were encountered: