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

Make whitespaces visible in GDScript code samples #3426

Open
YuriSizov opened this issue Apr 21, 2020 · 6 comments
Open

Make whitespaces visible in GDScript code samples #3426

YuriSizov opened this issue Apr 21, 2020 · 6 comments
Labels
content:website Issues related to adding website features and fixing bugs, whether on the front or back-end enhancement

Comments

@YuriSizov
Copy link
Contributor

Your Godot version:
Any

Issue description:
As evident from #3423, it can be hard to understand different indent sizes in code samples. It may be only limited to the GDScript style guide, because it directly talks about various cases for the indentation, but it may make code more readable for some overall. Which is important, since GDScript is indent-sensitive language.

Unfortunately, docs don't use tabs for indents, which is a problem in itself. I think this limitation is deeply related to how Sphinx works. Pygments can make both tabs and whitespaces visible, if we ever solve that.

For now, we have to work with whitespaces. Ideally, to be as least obstructive as possible, we'd like to have it as an option for ::code-block directive, but it has no such functionality. So, with a little hack, that is only going to work in Sphinx prior to version 3.x, we can enforce visible whitespaces on all gdscript code samples.

This must be added/changed in gdscript.py extension:

from pygments.filters import (
    VisibleWhitespaceFilter,
)
...
def setup(sphinx):
    l = GDScriptLexer()
    l.add_filter(VisibleWhitespaceFilter(spaces=" ",wstokentype=True))
    sphinx.add_lexer("gdscript", l)

And styles similar to this should be added in custom.css:

.highlight .w {
    position: relative;
}
.highlight .w:before {
    content: "·····································";
    position: absolute;
    left: 0;
    width: 100%;
    overflow: hidden;
    color: #4d566d;
    user-select: none;
}

And this is the result:

image

Note, that these characters are ignored when a user is selecting code samples.

However, not everyone likes visible whitespace characters. Forcing it globally may be undesirable. We can try and fix this particular example instead. It would be impossible to add any character in it and not break highlighting and copying. Some may say, that in this trivial example copying is not important. Highlighter errors can also be forcefully disabled.

As a middle ground, there is an option to only make those characters visible when used in a combination with emphasized lines:

image

To achieve this we need to make code blocks like this:

.. code-block::
    :emphasize-lines: 2

    for i in range(10):
        print("hello")

And adjust CSS rule to be .highlight .hll .w:before. Highlighted lines need a small fix as well, because we do not handle them at the moment. We just need to add a proper background color for them, like background-color: #2d3444;.

URL to the documentation page (if already existing):
https://docs.godotengine.org/en/stable/getting_started/scripting/gdscript/gdscript_styleguide.html

@aaronfranke
Copy link
Member

The convention for indentation in GDScript is that it should be indented using tabs, which are not spaces. The documentation should reflect this. Maybe something like this?

Good:

for i in range(10):
→   print("hello")

Bad:

for i in range(10):
→   →   print("hello")

@YuriSizov
Copy link
Contributor Author

YuriSizov commented Apr 23, 2020

@aaronfranke As I've noted, documentation does not use tabs. Sphinx converts everything to spaces. Wherever we actually have tabs in .rst files, they are converted into 8 spaces. Everywhere else docs are just formatted with 4 spaces instead of a tab, including this particular style guide.

If we fix that, my proposal can be adjusted to use a tabulation symbol instead of a middle dot. For now, we don't have such opportunity. Plus, the first example clearly uses a half-tab distance, which would still be made using a couple of spaces.

Edit: If you propose we just replace 4 dots with a tabulation sign and 3 spaces to imitate the proper tabulation, this is, of course, achievable, but may be harmful for people copying code samples.

image

@Calinou
Copy link
Member

Calinou commented Apr 23, 2020

As I've noted, documentation does not use tabs. Sphinx converts everything to spaces. Wherever we actually have tabs in .rst files, they are converted into 8 spaces. Everywhere else docs are just formatted with 4 spaces instead of a tab, including this particular style guide.

We could replace 4 spaces with tabs in code blocks using JavaScript when the page is loaded. (The CSS property tab-width: 4 can be used to adjust the tab width, as the default value 8 is too high.)

@YuriSizov
Copy link
Contributor Author

YuriSizov commented Apr 23, 2020

2020-04-23_17-39-24

Looks alright. This one line in custom.js should be enough:

document.querySelectorAll(".highlight span.w").forEach((item) => { item.innerText = item.innerText.replace(/[ ]{4}/g, "\t") })

Styles are something like this at this point:

.highlight .hll {
    background-color: #2d3444;
}

.highlight .w {
    position: relative;
    tab-size: 4;
}
.highlight .hll .w:before {
    content: "→   →   →   →   →   →   →   →   →   →   ";
    position: absolute;
    left: 0;
    width: 100%;
    overflow: hidden;
    color: #4d566d;
    user-select: none;
}

I can finalize all the changes and make a PR, if we agree on this solution.

Edit: Note that this will show the arrow for both spaces and tabs. If we want distinction with this solution, we have to add a bit more of JavaScript code, probably.

@Calinou
Copy link
Member

Calinou commented Apr 23, 2020

@pycbouh Sounds good to me, feel free to open a PR. Make sure to add a comment to explain why spaces are replaced with tabs in the JavaScript file.

Edit: Note that this will show the arrow for both spaces and tabs. If we want distinction with this solution, we have to add a bit more of JavaScript code, probably.

I think we should just write a comment in the GDScript style guide snippet that contains spaces, like:

# Note: In the snippet below, indentation displays a a tab, but the code snippet actually uses 2 spaces for indentation.

@mhilbrunner
Copy link
Member

Note: see discussion in #3437

@skyace65 skyace65 added the content:website Issues related to adding website features and fixing bugs, whether on the front or back-end label Dec 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content:website Issues related to adding website features and fixing bugs, whether on the front or back-end enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants