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

nim doc/rst2html renders \v, \\, etc incorrectly #17260

Closed
timotheecour opened this issue Mar 5, 2021 · 3 comments · Fixed by #17315
Closed

nim doc/rst2html renders \v, \\, etc incorrectly #17260

timotheecour opened this issue Mar 5, 2021 · 3 comments · Fixed by #17315
Labels
Documentation Generation Related to documentation generation (but not content).

Comments

@timotheecour
Copy link
Member

timotheecour commented Mar 5, 2021

Example 1

##[
foo1 `bar` `v` `\` `\v` `\\`

foo2 ``bar`` ``v`` ``\`` ``\v`` ``\\``
]##

Current Output

1st line incorectly rendered:
image

Expected Output

image

Example 2

ditto with rst2html:

foo1 `bar` `v` `\` `\v` `\\`

foo2 ``bar`` ``v`` ``\`` ``\v`` ``\\``

also renders incorrectly 1st line

Example 3

when true:
  ##[
  before (irrelevant comment just to show that the (buggy) error also has incorrect line/col info
  ]##

  ##[

  ==================         ===================================================
    Escape sequence          Meaning
  ==================         ===================================================
    `\"`                     foo1
    `\`                      foo2
  ==================         ===================================================
  ]##

gives: t11956b.nim(3, 18) Error: '`' expected
which is wrong for 2 reasons:

  • there should be no error
  • the error has wrong line/col info

Additional Information

/cc @a-mr

examples 4

the devel docs are broken: https://nim-lang.github.io/Nim/system.html#addEscapedChar%2Cstring%2Cchar
image

note:

inline in github:
foo1 bar v \ \v \\

foo2 bar v \ \v \\

@quantimnot
Copy link
Contributor

I'm not really familiar with RST and MD, but it seems to me that there are spec conflicts between RST inline interpreted text and MD inline code.

https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#inline-markup-recognition-rules
vs
https://github.github.com/gfm/#code-span

RST:

Both, inline markup start-string and end-string must not be preceded by an unescaped backslash (except for the end-string of inline literals).

GitHub MD:

A code span begins with a backtick string and ends with a backtick string of equal length.
The contents of the code span are the characters between the two backtick strings, normalized in the following ways:

First, line endings are converted to spaces.
If the resulting string both begins and ends with a space character, but does not consist entirely of space characters,
a single space character is removed from the front and back.
This allows you to include code that begins or ends with backtick characters,
which must be separated by whitespace from the opening or closing backtick strings.

Fixing this issue means not supporting RST inline interpreted text. Which also means the .. default-role:: code is not needed since the semantics are changed to match MD.

@timotheecour
Copy link
Member Author

timotheecour commented Mar 9, 2021

I don't think that's correct, inline markup applies inside a text block, not a interpreted text block (things inside a single backtick pair)

Furthermore, as you can see in https://github.com/quantimnot/Nim/blob/manual_rst_backtick_refactor/doc/manual.rst, after your PR the rst renders fine in github for

`\v` `vertical tabulator`:idx:

image

so it's just a bug specific to nim's rst to html renderer IMO; @a-mr can you confirm that this is also your understanding?

@a-mr
Copy link
Contributor

a-mr commented Mar 9, 2021

Hi @timotheecour . Yes, I think you are almost correct.

The RST spec does not cover escaping in interpreted text explicitly but the idea is mostly clear.

De facto behavior of python rst2html is this:

  • With some roles escaping with backslash works
  • with the code role it mostly does not. The only exception is for backslash+single back-tick: \ ` where it is still escaped. (The code role is not really documented about that behavior). Which actually makes sense for Nim also because we use single backticks in proc names.

It seems the rationale for this behavior is that interpreted text should be as flexible as possible — it can contain arbitrary syntax inside it. So handling of backslashes is completely up to the current role.

For general code role it's logical to insert backslashes literally but one case in your description is illegal

`\`

and should be still represented as

``\``

Summary: yes, we should fix our parser for the code role.

I can do it soon if you wish, I'm on vacation currently.

timotheecour added a commit to timotheecour/Nim that referenced this issue Mar 10, 2021
timotheecour added a commit to timotheecour/Nim that referenced this issue Mar 10, 2021
timotheecour added a commit to timotheecour/Nim that referenced this issue Mar 10, 2021
@a-mr a-mr mentioned this issue Mar 11, 2021
28 tasks
@ghost ghost added the Documentation Generation Related to documentation generation (but not content). label Mar 18, 2021
timotheecour added a commit to timotheecour/Nim that referenced this issue Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Generation Related to documentation generation (but not content).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants