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

Expose code block metadata to SyntaxHighlighterAdapter #257

Closed
wants to merge 11 commits into from

Conversation

lucperkins
Copy link
Contributor

@lucperkins lucperkins commented Dec 12, 2022

Fixes #253.

This PR is very much a WIP but I wanted to submit what I have so far in pursuit of a bit of help. In a nutshell, this PR exposes any code block "metadata" to the SyntaxHighlighterAdapter. Here's what I mean by metadata:

```rust showLineNumbers {1-5,11}

In this case, the string showLineNumbers {1-5,11} would be exposed via the meta key in attributes in the build_pre_tag function, as I've done in the syntax_highlighter.rs example. Run cargo run --example syntax_highlighter to see the respective attributes maps.

My goal is to enable people to build things like rehype-pretty-code in pure Rust. As far as I can tell, this is not currently possible in comrak via the SyntaxHighlighterAdapter. I've marked this as a draft because, as you can see, the actual implementation is pretty straightforward. What I'm less clear on is how you'd like this to fit in with the configuration options. I would love some guidance on what you'd prefer there. One possibility might be adding a distinct Boolean option to ComrakRenderOptions, like fancy_code_blocks or something, to capture both lang and meta, or maybe it'd be best to just capture the entire thing as one string, i.e. meta would be rust showLineNumbers {1-5,11} and users could do whatever they like with it.

@kivikakk
Copy link
Owner

kivikakk commented Jan 1, 2023

Hi Luc! Thanks so much for this, and sorry for my delay in responding! This is shaping up excellently.

What I'm less clear on is how you'd like this to fit in with the configuration options. I would love some guidance on what you'd prefer there. One possibility might be adding a distinct Boolean option to ComrakRenderOptions, like fancy_code_blocks or something, to capture both lang and meta, or maybe it'd be best to just capture the entire thing as one string, i.e. meta would be rust showLineNumbers {1-5,11} and users could do whatever they like with it.

I think we should simply always capture it and make it available. I'd prefer to avoid having a second boolean, because — as the match expression shows — understanding why certain attributes are emitted with certain combinations of options and highlighters becomes complicated fast. The actual data flow starts to get hard to track, too.1

I particularly want to avoid the "2 booleans (4 combinations) yet 3 code paths" we have here — it reveals the disconnect between the modelling of the configuration and the modelling of the logic proper. This would better be represented with a single tri-state option, but I figure we can afford to avoid changing the configuration entirely.

Instead, just always add the meta attribute (where there's relevant data) to pre alongside lang when github_pre_lang is specified. I also think it's best to leave meta as only what follows the language, i.e. just showLineNumbers {1-5,11} in your example.

If this becomes an issue for someone down the track, we can deal with it then, but for now simple is best.

Footnotes

  1. As an example: there is currently a bug that becomes exposed when you massage the tests back into service. The lang parameter passed to the highlighter has been refactored to re-use the pre_attributes hash. If you're not using either option, pre_attributes never gets anything added to it, and so the highlighter always gets None for the language.

@lucperkins
Copy link
Contributor Author

@kivikakk Okay, I've made those suggested changes such that meta is always provided with github_pre_lang is true. Let me know if you think this interface looks good to you and I'm happy to provide tests and docs and such.

@kivikakk
Copy link
Owner

kivikakk commented Jan 8, 2023

👍 Interface looks good to me! Uncomplicated is best.

@lucperkins
Copy link
Contributor Author

lucperkins commented Jan 9, 2023

@kivikakk These changes have introduced some test failures that I need to poke at 😅 I should be able to knock that out this week.

@kivikakk
Copy link
Owner

kivikakk commented Mar 28, 2023

Things have moved on a bit in the last couple of months -- we now have ComrakRenderOptions::full_info_string (from #276), which exposes the remainder of the info string to the highlighting plugin via the appropriate tag.

If it's alright with you, I'll close this PR and #253 -- I think Comrak now supports your use case.

@lucperkins
Copy link
Contributor Author

lucperkins commented Mar 28, 2023

Apologies for dropping off there. My life and employment situation changed pretty drastically. I'll close this, as I think that #253 indeed satisfies the use case 🚀

@lucperkins lucperkins closed this Mar 28, 2023
@lucperkins lucperkins deleted the syntax-metadata branch March 28, 2023 10:32
@kivikakk
Copy link
Owner

Not a problem at all, can totally relate! Glad to hear it; thanks for your efforts as always! 🐰🤍

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.

Add metadata to syntax highlighter
2 participants