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

Add support for tables and strike-through text in the XSLT #117

Merged
merged 4 commits into from
Sep 25, 2018
Merged

Add support for tables and strike-through text in the XSLT #117

merged 4 commits into from
Sep 25, 2018

Conversation

maelle
Copy link

@maelle maelle commented Sep 24, 2018

this PR adds support for tables and strike-through text to @nwellnhof's useful XSLT stylesheet.

The tables obtained are not "pretty", i.e. each column header gets three dashes only.

Cc @jeroen

@jeroen
Copy link

jeroen commented Sep 24, 2018

This is great! @nwellnhof would you have any suggestions for improvements?

@nwellnhof
Copy link

It looks like this won't work with table cells containing multiple lines. Supporting multiple lines probably requires a two-pass approach and the EXSLT node-set extension.

@nwellnhof
Copy link

I'd also suggest to put the templates handling GHF extensions in a separate stylesheet and import the core stylesheet with xsl:import. This way, you can simply overwrite the core xml2md.xsl file whenever it is changed upstream.

@maelle
Copy link
Author

maelle commented Sep 24, 2018

👋 @nwellnhof ! Thanks for your review!

  • Good point about the separate stylesheets, will do!
  • I'm afraid I do not understand what you mean by "a two-pass approach and the EXSLT node-set extension." Could you please provide pointers or an example? Thank you in advance!

@nwellnhof
Copy link

Thinking more about it, supporting multiple lines in table cells is anything but trivial. Sorry, but I don't have time to help you there.

@kivikakk
Copy link

We don't support table cells with multiple lines as it stands, so I think it's a non-issue.

@maelle Let me know when this is ready for merge (if you plan on separating out the extensions).

@maelle
Copy link
Author

maelle commented Sep 25, 2018

I've now made it a separate stylesheet. 😸

I didn't know multilines tables weren't supported, one problem less then.

I added a small comment inside the stylesheet to explain what it is.

Btw I was wondering whether/where the two stylesheets should be documented, i.e. how would someone know they even exist.

@kivikakk
Copy link

I've now made it a separate stylesheet. 😸

Awesome, thank you!

Btw I was wondering whether/where the two stylesheets should be documented, i.e. how would someone know they even exist.

This is a good point — @jgm, do you have any thoughts/opinions here? Right now xml2md.xsl is buried in the tools directory and no reference to it is made. I'm not a consumer of this, so I wouldn't know where I would go looking for documentation/pointers to such a thing if I was one.

@kivikakk kivikakk merged commit e7dc6ae into github:master Sep 25, 2018
@jgm
Copy link

jgm commented Sep 25, 2018 via email

@maelle
Copy link
Author

maelle commented Sep 26, 2018

@jgm I'd be happy to open a PR to the commonmark/cmark repo. Maybe a subsection of "Usage" in the README could be "Misc" and mention the stuff in https://github.com/commonmark/cmark/tree/master/tools including the stylesheet.

  • If I do that could I add a sentence about the complementary XSLT stylesheet living in the github fork?
  • What are the other files in tools/, are they worth mentioning in the README?
  • I'd also add a pointer to the docs of the XML format in another subsection at the same time if that's useful. :-)

talum pushed a commit that referenced this pull request Sep 14, 2021
* Add support for tables and strike-through text in the XSLT

* Update xml2md.xsl

* Add specific stylesheet for tables and strike-through

* Add comment
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

Successfully merging this pull request may close these issues.

None yet

5 participants