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

rst discussion for PR 17528 #673

Closed
timotheecour opened this issue Mar 27, 2021 · 2 comments
Closed

rst discussion for PR 17528 #673

timotheecour opened this issue Mar 27, 2021 · 2 comments

Comments

@timotheecour
Copy link
Owner

timotheecour commented Mar 27, 2021

@a-mr I know you closed nim-lang#17528 but just wanted to clarify a few things before moving on

it's universal: see the example above. (it even allows trailing whitespaces via syntax beginning of code \ , though that currently did not work in Nim)

to clarify, these still don't work after this PR (neither trailing backtick nor trailing space):

`foo\ `

`foo\``

give errors

furthermore, at least with rst2html and github, there is no good solution:

  • without .. default-role:: code, these would render as italics instead of as code, which is not what we want
  • without .. default-role:: code, even existing single backtick snippets that don't require escaping (eg foo) would not render well in github either, so we'd have to convert back all the single backticks to double backticks, reverting what we've been doing in the past few months (or, if only done in rst but not nim files, you'd have to remember to quote things differently in rst vs nim, which isn't good either eg when copy pasting etc)
  • on the other hand, with .. default-role:: code, the \ would be non-escaping so no possibility to escape backtick or single trailiing space etc

you wrote earlier:

Using code as the default role is fine. (the fact that python rst2html has title-reference, which is italic text, as the default role makes little sense to me.) But it means we should be able input backticks there with `.

note that it's not just rst2html, github also has this behavior

syntax highlighting

then there's the problem of syntax highlighting.
both rst2html and github render this:

`export FOO=\`pwd\``:bash

with the non-escaping backlash, showing both the backslash and the backtick:

image

image

in the end, there is no good standard solution, hence RFCs/issues/355 which doesn't have any edge case and allows optional syntax highlighting wihtout requiring escaping.

@a-mr
Copy link

a-mr commented Mar 27, 2021

you'd have to remember to quote things differently in rst vs nim

Yes, a fair point, it's actually the main reason why I closed nim-lang#17528

Regarding

`export FOO=\`pwd\``:bash:

I'm hesitating to tell you, but recently I realized that actually there is a way in the original RST to render this correctly:

`export FOO=`pwd``:bash:

It works in rst2html.py because there is no whitespace after 2nd and 3rd `. It's one of RST inline markup rules.

The problem with that syntax is that it's very fragile. Add ; after 3rd ` and it gets broken.

So our current interpretation of \` is more robust.

We almost conform with the letter and spirit of RST spec, because

  • it does not say explicitly about need to escape in interpreted text in the main spec. The only place where it is mentioned is roles list: literal role doc
  • however it includes the following rule:

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

(inline literals are double backticks)
By keeping escaping for backticks in nim-lang#17315 you made our implementation conform with this rule. The fact that it's rendering literally in rst2html.py with :code: is not in the spec, so we are good :-) Thank you

My current thoughts is that we should leave it as is. And don't implement escaping for literal role — just write it in rst.nim: Limitations. Because Nim is supposed to be simple. Forked rules for interpreted text is an unnecessary complication IMHO.

@timotheecour
Copy link
Owner Author

good that we've converged to an agreement on this!

nim-lang/RFCs#355 will solve all remaining cases where current escaping rules fall short, but this should be discussed in that rfc

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

No branches or pull requests

2 participants