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

sgr_to_html() adds unneeded <span> #69

Closed
hadley opened this issue Apr 27, 2021 · 4 comments
Closed

sgr_to_html() adds unneeded <span> #69

hadley opened this issue Apr 27, 2021 · 4 comments
Milestone

Comments

@hadley
Copy link

hadley commented Apr 27, 2021

x <- "<div>\033[31mx\033[39m y</div>"
fansi::sgr_to_html(x)
#> [1] "<div><span style='color: #BB0000;'>x</span><span> y</div></span>"

Created on 2021-04-27 by the reprex package (v2.0.0)

I think the result should be:

#> [1] "<div><span style='color: #BB0000;'>x</span> y</div>"

?

@brodieG
Copy link
Owner

brodieG commented Apr 27, 2021

It's to simplify the string writing logic, since it's in C and a pain:

https://github.com/brodieG/fansi/blob/master/src/tohtml.c#L177

Also, there is some interaction with #59, apparently:

https://github.com/brodieG/fansi/blob/master/src/tohtml.c#L503

Obviously the example you provide is not nice, but are you really dealing with mixed SGR/HTML? I can contemplate doing this cleaner, but there would have to be a compelling case to so since it's going to be annoying. Obviously conceptually it's simple, but making sure I don't introduce any subtle bugs in the process will take some work.

@hadley
Copy link
Author

hadley commented Apr 27, 2021

It's not a big deal; I've worked around it from end by doing the SGR conversion earlier.

@brodieG
Copy link
Owner

brodieG commented Apr 27, 2021

Ok, so I probably won't do anything about it, but will keep it open for whenever I get around to the css class thing, as that would be a natural moment to tackle this as well.

brodieG added a commit that referenced this issue May 23, 2021
* Reduce differences between read and write.
* Stricter tracking of bytes written.
* Remove size calc wrapper.
* Reduce spurious HTML.
@brodieG brodieG added this to the 0.5.0 milestone May 24, 2021
@brodieG
Copy link
Owner

brodieG commented May 25, 2021

0.5.0 produces more efficient html.

Do note you probably should html_esc the input as even > or < will mess up the HTML. I'm considering having that be the default, but I'm a bit worried I'll break code that manually escapes, so might not make that change.

@brodieG brodieG closed this as completed May 25, 2021
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Jun 5, 2021
# 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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants