godotengine / godot-docs

Godot Engine official documentation
https://docs.godotengine.org
Other
3.81k stars 3.09k forks source link

Unwanted styles are being applied to code blocks by the `.highlight` class #6931

Open jwmcgettigan opened 1 year ago

jwmcgettigan commented 1 year ago

Your Godot version:

Latest (4.0)

Issue description:

The current .highlight styles appear to be applying styles for a specific lexer/language (presumably shell) to all code-blocks regardless of their language. Instead, language specific styles should be set in their language specific highlight class (e.g. .highlight-shell and .highlight-powershell).

Example: .. code-block:: batch command treated as comment.

image

<div class="highlight-batch notranslate">
  <div class="highlight">
    <pre>
      <span></span>C<span class="p">:</span><span class="nl">\godot</span><span class="c1">&gt; scons platform=windows target=template_debug arch=x86_32</span>
      C<span class="p">:</span><span class="nl">\godot</span><span class="c1">&gt; scons platform=windows target=template_release arch=x86_32</span>
      C<span class="p">:</span><span class="nl">\godot</span><span class="c1">&gt; scons platform=windows target=template_debug arch=x86_64</span>
      C<span class="p">:</span><span class="nl">\godot</span><span class="c1">&gt; scons platform=windows target=template_release arch=x86_64</span>
    </pre>
  </div>
</div>

image

The highlight class and child classes

https://github.com/godotengine/godot-docs/blob/361bdfc6d000172df5d46913cce4fbcaaba0f0e5/_static/css/custom.css#L820-L924

URL to the documentation page (if already existing):

This section is shown in the above example.

YuriSizov commented 1 year ago

We use a shell syntax highlighter there, but it's not designed for Windows-style path, hence the issue. If there is a cmd syntax highlighter as an alternative, we could use that. Or we could properly escape paths with double slashes.

And the first example will always be invalid, because it shows both the commands and a shell context (i.e. the current system path), which is not a part of the syntax.

jwmcgettigan commented 1 year ago

We use a shell syntax highlighter there, but it's not designed for Windows-style path, hence the issue. If there is a cmd syntax highlighter as an alternative, we could use that. Or we could properly escape paths with double slashes.

And the first example will always be invalid, because it shows both the commands and a shell context (i.e. the current system path), which is not a part of the syntax.

Sorry, I'm not sure I understand what you mean. Can you elaborate?

YuriSizov commented 1 year ago
  • Are you configuring it to be a 'shell' syntax highlighter somewhere is that simply the default?

It's set as a page default using the .. highlight:: shell line at the top, yes. And yes, Pygments is responsible for highlighting.

  • I've tried using double slashes without success.

Then I guess we can't do that.

Sorry, I'm not sure I understand what you mean. Can you elaborate?

Some command line examples in the docs, including the one in your first image, include not only the command itself, but also the part that you see in the terminal, such as the name of the shell or the path in the local system. In this case it's C:\godot>. This is done as a reference for the reader, I guess, but it's not a part of the valid syntax for bash shells or any other standard. So because of that it will always cause errors in the highlighter.

jwmcgettigan commented 1 year ago

Pygments has Windows related lexers such as batch and powershell. But they each have their own drawbacks compared to shell.

I'm looking into creating a custom lexer like the GDScriptLexer to handle:

YuriSizov commented 1 year ago

That would be an overkill for this minor issue. Maintaining our own lexers as a workaround of shell syntax highlighting is out of scope for the project.

We can remove the context part, as it can be confusing anyway. And for slashes we can probably use forward slashes on all platforms. Most tools should be fine with that.

jwmcgettigan commented 1 year ago

I've become much more familiar with Sphinx since I created this issue and so I have a better idea how to approach the problem.

I think that this issue is actually two issues:

  1. The current .highlight styles appear to be applying styles for a specific lexer/language (presumably shell) to all code-blocks regardless of their language. Instead, language specific styles should be set in their language specific highlight class (e.g. .highlight-shell and .highlight-powershell).
    • This issue is why I previously mentioned that the batch and powershell lexers also had drawbacks. They both are having unwanted styles applied to them from the blanket .highlight class.
    • Until this issue is addressed, a good solution to prevent unwanted styling is to utilize .. code-block:: none which will prevent any styling from being applied to a code-block.
  2. We should improve the documentation for contributing to godot-docs to include a basic guide to code-blocks.
    • Mention Pygments and add a link to the lexers page as there is currently no mention of Pygments in the docs.
    • Explain how to provide language specific styles if trying to add style support for new language specific code-blocks.
    • Explain how to prevent unwanted styling in code blocks

Given the points above, I've opted to rename this issue to "Unwanted styles are being applied to code blocks by the .highlight class" to address the first issue from above. I've updated the contents of the original post as well.

For the second issue above, I've created https://github.com/godotengine/godot-docs/issues/7016.

YuriSizov commented 1 year ago

The way the highlighter works is it tokenizes the code, breaks it up into general-concept tokens, such as "identifier", "number", or "comment". Plus a few permutations of the concept to handle more complex cases. That's why all c, c1, cm, and cs are using the comment color. These CSS classes mean the same thing in any language, so they can be styled the same way, and we don't need to define some specific rules like .highlight-powershell .c1.

Again, the lines starting with C:\godot> break highlighting because they are not valid lines of any code.

jwmcgettigan commented 1 year ago

Thank you for clarifying that for me. 🙏

If that is the case, then it sounds like the solution to this issue is to be aware and mindful of these rules that dictate how code-blocks tokenize the code and what styles are applied. This leads me to believe that https://github.com/godotengine/godot-docs/issues/7016 is likely sufficient enough to address what lead me to create this issue.

Takeaways

  1. Contributors must manually/visually verify that the content of the code block is supported by the lexer being used.
  2. Is there a way to validate whether the content of a code-block is supported by its lexer?
    • If there is, we could potentially add a code-block validation step to the building of the docs. This would throw errors to inform the user that a particular code-block's contents are not supported by the lexer it is using. This would prevent the human error responsible for this issue at its roots.

What do you think? While takeaway two may very well be overkill depending on the amount of effort required to implement it, I'm curious as to whether you think it is viable in the first place?

YuriSizov commented 1 year ago

Is there a way to validate whether the content of a code-block is supported by its lexer?

It depends on the lexer. When the pages are built, it will err if it cannot parse the content at all (and I think the build itself will fail then). But the lexer can also be implemented in a recoverable way, and keep parsing through invalid syntax. The only recourse there is to disable highlighting for the block all together (with none), and perhaps leave a note for the future.

For example: https://github.com/godotengine/godot-docs/blob/da46470513dec6022eb75c68d19e539f6c142e4a/tutorials/shaders/shaders_style_guide.rst?plain=1#L349-L360

Overall, yes, contributors should validate the look of the page they edit. But we are also lenient with documentation contributions and don't require anyone to build the docs locally to test. Many contributions are done through the GitHub's web editor tools, and that's fine.