-
-
Notifications
You must be signed in to change notification settings - Fork 978
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
Support pandoc headerless table in table formatting #1895
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with this PR. Let's wait and see what @fkohrt feels about the JS solution at #1893 (comment).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just make this change. I guess it should be safe. If it breaks anything, users will still have the ability to .removeClass()
on the tables that shouldn't have been styled, so it won't be the end of the world. Thanks!
5d61795
to
eed2f6c
Compare
Great ! I rebased. We wait for CRAN release before adding a bullet to NEWS and merging |
v2.4 is on CRAN now: https://cran.r-project.org/package=rmarkdown |
eed2f6c
to
894ca72
Compare
Thanks ! I rebased again and added a NEWS bullet. Feel free to reword in master if necessary. |
I find this one easy to fix #1893 but I agree that this script exists from some time now (#1893 (comment))
pandoc table can have no header so we could point the selector toward another pandoc table specific class.
See https://github.com/jgm/pandoc/blob/9cad5499c45022b9893156bb5e0468cabefca974/src/Text/Pandoc/Writers/HTML.hs#L954
pandoc adds
header
class totr
in table header but it also addodd
(oreven
) as class intr
insidetbody
. We could use that to target pandoc table. But maybe that is more common class addition than the previous selector.As you suggested @yihui we could not add support for that and leave the user customise itself using JS (but it must know how...)
If we want to continue with this PR, I'll add some more tests to be sure.