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

Website markdown does not render C++ code blocks with C++ highlighting #305

Closed
hyp opened this issue May 17, 2023 · 10 comments
Closed

Website markdown does not render C++ code blocks with C++ highlighting #305

hyp opened this issue May 17, 2023 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@hyp
Copy link
Contributor

hyp commented May 17, 2023

While working on #299, I noticed that the markdown code I added to represent a C++ sample, is not rendered with C++ highlighting on my local instance on the website.

Please support specifying a language like C++ in the code blocks, using Github's syntax, e.g.:

```c++
@hyp hyp added the bug Something isn't working label May 17, 2023
@alexandersandberg
Copy link
Member

Thanks for bringing this up, @hyp!

So it seems like the syntax highlighting actually does work, however, it's not as good as GitHub's highlighter.

Without highlighting With highlighting GitHub's highlighting
CleanShot 2023-05-22 at 09 08 51 CleanShot 2023-05-22 at 09 09 58 CleanShot 2023-05-22 at 09 10 20

According to rouge's list of supported languages, cpp (alias c++) is supported.

I had a look at the HTML that the highlighter renders and noticed that it actually highlights more code than we think.

CleanShot 2023-05-22 at 09 18 18

So the issue is actually that some CSS classes are missing from _syntax.scss!

I don't know who put this CSS file together, but I will have a closer look at resolving this issue soon.

@ishaanbedi
Copy link
Contributor

Hey @alexandersandberg!

I'd like to work on this issue.

As you suggested that some classes are missing from the scss file. The approach I'd like to follow is to explicitly initialize the classes and their corresponding styling effects for those classes exclusive to CPP (such as .nl) that haven't been initialized in the scss file yet but are present in cpp code blocks across the website.

There are 44 such instances of "```c++" code blocks and 1 instance for "```cpp", and most of the classes are common & overlap with each other among these instances, so manual scanning and working on them would not be a difficult task.

I don't have much experience using rouge, but I had a quick glance at the repo and could not find any information about CPP classes or styling instructions, if they provide. If you could help me by letting me know the best way to proceed, I would highly appreciate it.

@ishaanbedi

@alexandersandberg
Copy link
Member

Thanks for wanting to help out, @ishaanbedi!

IIRC the problem here is that there's no theme provided by rouge that matches the theme we use on the website, so it's not as simple as copy-pasting over the missing values from somewhere.

Instead, we might have to look at existing rouge themes to see how the individual colors are distributed across different classes and try to replicate those patterns to our theme.

However, this color distribution might differ between themes, so we also need to do some trial and error here to in the end make the C++ syntax match the rest of the website's syntax.

I think it would be great to set up a little test page (temporarily, which will be removed later) with a few code snippets in C++ and maybe one or two other languages that contain as many syntax highlighting classes as possible. This can then be used while evaluating whether a color suits a certain class or not.

Does that make sense?

@ishaanbedi
Copy link
Contributor

Hi @alexandersandberg !

That does sound right.

Instead, we might have to look at existing rouge themes to see how the individual colors are distributed across different classes and try to replicate those patterns to our theme.

I was able to extract css files for the themes supported by rouge here and I can see it has various classes which are not declared in the _syntax.scss file for the website, so yeah I can get references from inspecting them and comparing what all is missing, and replicating them based on the theming styles and guidelines of the website as per the theming and colors for other code blocks, just like you mentioned.

Also, this website https://spsarolkar.github.io/rouge-theme-preview/ does provide a great overview about pre-existing rouge themes, so I can get help from this as well.

I think it would be great to set up a little test page (temporarily, which will be removed later) with a few code snippets in C++ and maybe one or two other languages that contain as many syntax highlighting classes as possible. This can then be used while evaluating whether a color suits a certain class or not.

Sounds great, I'd like to give it a shot. For this, I'll open a draft PR while pushing commits on the go, since this will require deployments so as to see the colors and theming.

@alexandersandberg
Copy link
Member

Perfect, thank you!

Sounds great, I'd like to give it a shot. For this, I'll open a draft PR while pushing commits on the go, since this will require deployments so as to see the colors and theming.

Sadly, we're still in the process of setting up preview deployments for PRs, but you should be able to run the website locally and see the changes there.

@ishaanbedi
Copy link
Contributor

Sadly, we're still in the process of setting up preview deployments for PRs, but you should be able to run the website locally and see the changes there.

Oh, okay! I have set up the website environment locally and deployed it over here: https://swift-org-website.ishaanbedi.in/

Also, I have created an example page as you suggested: https://swift-org-website.ishaanbedi.in/issue305, and I have added a code example across C++, Swift & JS, to compare and work on the classes over there.

I'll keep you updated about the changes or progress I'll be making!

@ishaanbedi
Copy link
Contributor

Hi @alexandersandberg !

I was able to make some progress with the issue, and so far, it looks like:

Before After

I tried my best to match the theming of these blocks with the rest of the website and compared Swift & JS code blocks side by side for the same.

You can have a look at it here: https://swift-org-website.ishaanbedi.in/issue305
Or compare side by side between examples at: https://www.swift.org/documentation/cxx-interop/ & https://swift-org-website.ishaanbedi.in/documentation/cxx-interop/

I would like to know your feedback on this, or any suggestions you might have.

@alexandersandberg
Copy link
Member

This looks like a great improvement already! Feel free to open a draft PR and I can take a closer look at the changes and see if I have any suggestions for further improvement. 🙂

@ishaanbedi
Copy link
Contributor

Hi @alexandersandberg !
I've opened a draft PR as per your suggestion here.

@federicobucchi
Copy link
Contributor

This has been merged. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants