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

Classed syntax highlighting + Line numbers #1242

Closed

Conversation

evan-brass
Copy link
Contributor

@evan-brass evan-brass commented Nov 24, 2020

CSS classed syntax highlighting

This extends #913
ClassedHtmlGenerator uses tokens_to_classed_spans internally which creates a new scope stack for each line causing the panic when a restore occurs on a different line from a clear.

  • Documentation

    • Explain how to style classed highlighting (.code class, samples for <mark>, etc.)
  • Implement hl_lines (<mark> tags?)

  • Implement line numbers

    • Table
    • linenostart fence option
  • Tests

  • Is any more documentation needed?

  • Is it ok for highlighting_css_themes to not be under the new markdown section?

@evan-brass evan-brass force-pushed the classed-highlighting branch from 9e94065 to 5155f5b Compare December 4, 2020 02:15
@evan-brass
Copy link
Contributor Author

Hello @Keats. Is this something you're interested in? Before I continue, I wanted to see if this is going in a right direction.

@Keats
Copy link
Collaborator

Keats commented Dec 4, 2020

Sorry I thought I replied to it already! This is something we want but I'm wondering if we need to override the HTML syntect outputs so we can support line numbers and highlight in the best possible way (https://zola.discourse.group/t/highlighting-improvements/19/11?u=keats)

@evan-brass

This comment has been minimized.

@Keats
Copy link
Collaborator

Keats commented Dec 4, 2020

  1. Character highlighting: I don't think we want to do that, just lines are fine.
  2. Higher contrast: if we rewrite the HTML output, we can use mark as you say for highlighted text and let people overwrite the style via CSS. I'm not sure about using mark for syntax instead of span though. Why do you say it will reduce duplication?
  3. Line numbers: I believe to have it nicely working (copy/paste without the line numbers etc), we need to rewrite the HTML from syntect completely.

I think if we go the rewrite html route, adding the classes is trivial since we just put it instead of inline css?

@evan-brass evan-brass marked this pull request as draft December 5, 2020 00:32
@Keats
Copy link
Collaborator

Keats commented Dec 5, 2020

I think if we go the rewrite HTML route, the first thing to do is to write by hand the HTML we would like to end up so we can experiment easily

@evan-brass evan-brass changed the title Classed syntax highlighting Classed syntax highlighting + Line numbers Dec 8, 2020
@evan-brass evan-brass marked this pull request as ready for review December 8, 2020 23:38
@Keats Keats mentioned this pull request Jan 3, 2021
4 tasks
@uwearzt
Copy link
Contributor

uwearzt commented Jan 3, 2021

@evan-brass Thanks for pushing my PR forward. This year i have time to work on Zola (and my website). If you need any help, just drop me a note.

evan-brass and others added 17 commits January 6, 2021 02:19
Copied and pasted from pull request 913

Co-Authored-By: Uwe Arzt <[email protected]>
Mostly copied from PR 913

Co-Authored-By: Uwe Arzt <[email protected]>
This is copied from getzola#913

Co-Authored-By: Uwe Arzt <[email protected]>
Give `CodeBlock` access to the start and end events because classed highlighting needs to adjust the pre element and close spans.
Rename get_highlighter and make it return everything needed for highlighting so that we don't need the config after a codeblock is created.
Prep the code block for classed highlighting which will need to store different state from inline highlighting.
still need to do open_spans and lots of stuff.
* Remove html_escape: use Tera
* Output table
The foreground color will now be applied to the line numbers.  This also reduces the number of spans required in inline output.
@evan-brass evan-brass force-pushed the classed-highlighting branch from 59bc44d to 6d7f708 Compare January 6, 2021 11:35
@evan-brass evan-brass marked this pull request as draft January 6, 2021 11:51
@evan-brass
Copy link
Contributor Author

Sorry for the delay.

I'm still not sure what line number style to implement so I implemented the few discussed already. There's some bugs and no tests yet.

I don't like how much code I wrote in the codeblock module - it feels like there's some simple design that I'm not seeing. I tried to break the line numbers, highlighted lines, and syntax highlighting into multiple "passes", but it still doesn't seem right.

@Keats Keats closed this Jan 9, 2021
@Keats Keats deleted the branch getzola:syntax-highlight-++ January 9, 2021 13:51
@Keats
Copy link
Collaborator

Keats commented Jan 9, 2021

I did it again sorry >_>

@Keats Keats reopened this Jan 9, 2021
@Keats
Copy link
Collaborator

Keats commented Mar 6, 2021

@evan-brass is it ready or is it still a WIP?

@Keats
Copy link
Collaborator

Keats commented May 17, 2021

Another ping @evan-brass
I'll pick it up later this month otherwise

@kzurawel
Copy link

I've been using this branch for a while and so far I'm quite happy with it, but I had one suggestion/request: when using the "css" highlight theme, it would be nice to include the language-* classname from the <code> block on the <pre> tag as well. This would allow for doing things like setting different background colors for different types of code block (in my use case, allowing "plain text" fenced code blocks to blend in more with the page by having no background color, while allowing specific language code blocks to be more visually defined).

@Keats Keats marked this pull request as ready for review June 7, 2021 19:04
@Keats Keats changed the base branch from next to syntax-highlight-++ June 7, 2021 19:07
@Keats
Copy link
Collaborator

Keats commented Jun 7, 2021

It looks really good! Merge conflict is going to be a bit annoying (hidden lines, how it interacts with line numbers etc) but I'll try to merge things through the syntax-highlight-++ branch this week or so

@Keats
Copy link
Collaborator

Keats commented Jun 14, 2021

Erhm the merge is annoying, do you mind if I do it manually in a new commit and mark you as the commit author @evan-brass ? I will probably tweak some bits but most of it will be this PR.

@evan-brass
Copy link
Contributor Author

Sure. Sounds good.

@Keats
Copy link
Collaborator

Keats commented Jun 16, 2021

@kzurawel should the lang be moved entirely to pre instead of code? It's a bit silly to repeat the information 3-4 times

@kzurawel
Copy link

I would be fine with that; I can’t think of a situation where having the inner lang class would matter

@Keats
Copy link
Collaborator

Keats commented Jun 17, 2021

It's if you want to use something like highlightjs. I think I will just the class on pre and code actually so it covers everything.

@Keats Keats mentioned this pull request Jun 20, 2021
@Keats Keats force-pushed the syntax-highlight-++ branch from 6075d56 to 7b75ad0 Compare June 21, 2021 08:56
@Keats
Copy link
Collaborator

Keats commented Jun 21, 2021

Not finished yet but #1531 is a simplified implementation as it doesn't try to unify various spans, which sounds like it would something better done in syntect directly.
However the class based highlighting panics when ran on the Zola site despite the code being 99% copy/pasted from syntect itself so I might copy the code from this PR instead if I can't figure it out.

@Keats
Copy link
Collaborator

Keats commented Jun 22, 2021

trishume/syntect#336 and I made a PR to fix it so it should be ok

@Keats Keats force-pushed the syntax-highlight-++ branch from 7b75ad0 to ebfbd8d Compare July 2, 2021 18:30
@Keats
Copy link
Collaborator

Keats commented Jul 2, 2021

@kzurawel can you try #1531 ?

@Keats Keats closed this Jul 10, 2021
@Keats Keats deleted the branch getzola:syntax-highlight-++ July 10, 2021 06:49
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.

4 participants