Add support for highlighting code blocks using highlight.js.#54
Add support for highlighting code blocks using highlight.js.#54markcho wants to merge 1 commit intojfernandez:mainfrom
Conversation
jfernandez
left a comment
jfernandez
left a comment
There was a problem hiding this comment.
Nice addition - syntax highlighting is one of the most useful things for agent-produced content. A few things to address before merging.
| <link rel="stylesheet" id="hljs-theme" href="https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.11.1/styles/github.min.css"> | ||
| <script src="https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.11.1/highlight.min.js"></script> | ||
| <script src="https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.11.1/languages/scala.min.js"></script> | ||
| <script> |
There was a problem hiding this comment.
The highlight.js core already includes ~40 common languages (Python, JS, Rust, Bash, etc.). This line loads only Scala, which seems like a personal addition. Remove it, or if there's a specific reason for it, add the languages agents commonly produce instead.
There was a problem hiding this comment.
Working in the data space, Scala is still quite common. That being said, I understand why you wouldn't want every language listed here.
How about a parameter that controls which languages are included? Something like: --highlightjs=scala,foo,bar if you want to include support for languages that are not in the main pack.
There was a problem hiding this comment.
The rendered HTML already contains class="language-scale" or whatever the language used. In the server-side logic, you can extract the language names from the HTML and then pass them to the template.
Then you can do something like this in the template:
<script src=".../highlight.min.js"></script>
{% for lang in languages %}
<script src=".../languages/{{ lang }}.min.js"></script>
{% endfor %}| {% endif %} | ||
|
|
||
| <!-- Highlight.js for syntax highlighting --> | ||
| <link rel="stylesheet" id="hljs-theme" href="https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.11.1/styles/github.min.css"> |
There was a problem hiding this comment.
Mermaid is gated behind {% if mermaid_enabled %} so it only loads when the markdown actually contains diagrams. Consider doing the same here. The server already checks for class="language-mermaid" in the rendered HTML to set that flag; a similar check for any class="language- that isn't mermaid would let you conditionally load highlight.js only when code blocks are present.
| const themeLink = document.getElementById('hljs-theme'); | ||
| if (themeLink) { | ||
| themeLink.href = `https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.11.1/styles/${themeName}.min.css`; | ||
| } |
There was a problem hiding this comment.
The CDN base URL and version 11.11.1 appear in 4 places. Minor, but worth extracting to a variable to make future version bumps easier.
|
@markcho do you want to continue working on this PR? If not, I can take over too. |
2 participants
Add support for highlighting code blocks using highlight.js.