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 minor bugs #17340

Open
14 of 28 tasks
a-mr opened this issue Mar 11, 2021 · 12 comments
Open
14 of 28 tasks

RST minor bugs #17340

a-mr opened this issue Mar 11, 2021 · 12 comments
Labels
Documentation Generation Related to documentation generation (but not content).

Comments

@a-mr
Copy link
Contributor

a-mr commented Mar 11, 2021

Let us track minor bugs in our RST implementation here.

Details

[x] 1. prefix role syntax doesn't work. Only postfix syntax does.

See :subscript:`some text` and :code:`other text`

See `some text`:subscript: and `other text`:code:

image

Should be (rst2html.py):
image

When started at the 1st column it's not even parsed:

:subscript:`some text`

Error: ':' expected

[x] 2. directive default-role does not do anything

This does not make text subscript:

.. default-role:: subscript

See `some text`.

rst2html.py:
image

[x] 3. make code default role

This implies non-escaping backslashes between ` `.

Also we need to create a variable to track the default role in the first place :-)

[ ] 4. escape \ does not work as expected in references

(see also #17315 (comment))

HTML Element: <a>
-----------------

HTML Element: <b>
-----------------

HTML Element: <c>
-----------------

See `HTML Element: \<a>`_

This is not even parsed:

See `HTML Element: <b\> `_
See `HTML Element: <c>\ `_.

Error: '`' expected

[x] 5. indentation bugs: some elements don't work on the first line when nested

:field: #. Item1
        #. Item2

image

Should work (rst2html.py):
image

The same goes e.g. for bullet list as contents and admonitions as the container

Generally, indentation handling is not aligned/systematic in different body elements handlers.

[x] 6. directive include does not work if :literal: has indent < 3

E.g. indentation of :literal: is 2:

.. include:: grammar.txt
  :literal:

Then contents of grammar.txt are not included and literal: appears as a block quote.

[x] 7. simple tables with spanning cells are displayed wrongly

=====  =====  ======
   Inputs     Output
------------  ------
  A      B    A or B
=====  =====  ======
False  False  False
=====  =====  ======

image

Should be (rst2html.py):
image

[ ] 8. wrong line in footnote reference and substitution reference

Consider this 2 mistakes:

Check [2]_ and |vrsn|

.. [1] footnote

.. |version| replace:: 1.5

The error reported is for line 6, which is EOF.

Solution: preserve original line of the references.

The same problem will be relevant for smart links

[ ] 9. no warning if no empty line after or before directive

.. warning:: Unsuitable for benchmarking (but still better than `now`),
use `monotimes.getMonoTime` instead or `cpuTime`, depending on use cases.

Solution: report a warning like rst2html.py does:

(WARNING/2) Explicit markup ends without a blank line; unexpected unindent.

When there is no empty line before directive the spec does not require anything. It silently parses as a normal paragraph both in rst.nim and rst2html.py. But it can be really confusing, we have to add a warning for this case also.

Also directive requires a blank line after it:

Good:

.. code:: c

   int main

Bad:

.. code:: c
   int main

[x] 10. combining of bullet lists with option lists is sometimes broken

Consider an option list with more than 1 items and description on more than 1 lines. It's included into a bullet list.

* a
  -m   desc1
  -n   very long
       desc2
* b

The bullet list is broken.
image

Interesting, rst2html.py (v 0.14) is broken in another way: it complains about wrong indentation at desc2, the same option list without the bullet list parses OK.

[ ] 11. some valid field lists are not parsed

It is probable that presence of non-alphanumeric symbols breaks them:

:(empty):         current directory `.` is assumed (not with --replace)
:-:               read buffer once from stdin: pipe or terminal input;

[ ] 12. Markdown: \| inside a table cell should render as | not \|

\| outside a table cell should keep rendering as \| (as it already does since #17315)

13. Syntax for trailing whitespaces does not work

:literal:`hi \ `

[x] 14. backslash + backtick escape does not work at the end of interpreted text

If \` is right before closing backtick then it fails to parse:

Markdown syntax is `\`\`\`nim ... \`\`\``.

Error: '`' expected
Interestingly, adding characters makes it compile:

Markdown syntax is `\`\`\`nim ... \`\`\`x`.

[ ] 15. no warning when literal block has wrong indentation

* two immediate following underscores `__` are not allowed::

  letter ::= 'A'..'Z' | 'a'..'z' | '\x80'..'\xff'

image

rst2html.py gives a warning:
literalBlock.rst:3: (WARNING/2) Literal block expected; none found.

Nim silently ignores the mistake.

[x] 16. roles with valid refname symbols are not parsed

e.g. :c++: is not parsed:

`#include <stdio.h>`:c++:

UPDATE: c++ is not a valid name according to the spec, it's just that rst2html.py parses it. However, valid refnames are not recognized.

[x] 17. two dots without space after them are recognized as a comment

..note:: text...

disappears.

Found in #17671.

[x] 18. Only = should be allowed for borders in simple tables

Our implementation should not allow using - for borders in simple tables. It's against the spec and rst2html and Github don't parse such tables: https://github.com/nim-lang/Nim/blob/devel/doc/nep1.rst

[x] 19. Multi-line comments starting from line of .. not recognized

.. comment beginning
   continuation

image

[ ] 20. a blank line between options splits option list

e.g.

-a  desc1

-b  desc2

is parsed as 2 option lists.

[x] 21. code block with line indented after option disappears

.. code-block:: nim
   :number-lines: 35
   
    let a = 1

Line let a = 1 indented positively w.r.t. :number-lines: option and the whole code block just disappears.

[ ] 22. Markdown code blocks gobble up indentation on first line

```nim
  discard 1
  discard 2
```

image

May be related to #17314 .

[ ] 23. Markdown code blocks don't respect indentation rule

This

See code:

  ```
  some code
  ```

is displayed as

image

Note two spaces before some code and RST block quote quotation bar to the left of it.

https://spec.commonmark.org/0.29/#fenced-code-block

If the leading code fence is indented N spaces, then up to N spaces of indentation are removed from each line of the content (if present).

Probably RST block quote feature should be turned off in default nim mode for that to work.

[x] 24. Markdown links don't not allow balanced parentheses or escaped parentheses

  https://en.wikipedia.org/wiki/APL_(programming_language)

  [foo](https://en.wikipedia.org/wiki/APL_(programming_language))
  
  [foo](https://en.wikipedia.org/wiki/APL_\(programming_language\))
  
  [foo](https://en.wikipedia.org/wiki/APL_\(programming_language\))

According to the spec this should be allowed.

[ ] 25. docgen: multiple 1-level doc comments are glued without spacing

## Main text.

proc f*() = discard

## More text.

image

I would expect a blank line between "Main text" and "More text".

[ ] 26. Internal anchor cannot be set for enum.list/bullet list points

This is valid syntax for assigning an anchor to internal point of enumeration list or bullet list:

1. point1

   .. _lnk:
2. point2

Ref lnk_.

It works in rst2html.py but not in rst.nim.

[ ] 27. Text after 3rd backtick in code disappears

`proc `=destroy`[T](f: var Foo[T])`

image

Found in #19058

[ ] 28. Still incorrect handling of indentation when text is wrapped

.. Note:: #. a
   #. b

Current behavior:
image

Correct behavior by rst2html.py:
image

version

Nim 1.5.1, but all or almost all are applicable to stable versions.

cc @narimiran @timotheecour

@timotheecour timotheecour added the Documentation Generation Related to documentation generation (but not content). label Mar 11, 2021
@timotheecour
Copy link
Member

timotheecour commented Mar 11, 2021

make code default role

that makes sense for nim files; for rst files, I needed to add .. default-role:: code explicitly in #17028 so that it renders as expected in external tools like github. So we could also make nim rst2html implicitly assume .. default-role:: code (but honor ``.. default-role:: xxx), and document that the rst also needs .. default-role:: code` on top to ensure it also renders as code when viewed directly in tools like github.

wdyt ?

@a-mr
Copy link
Contributor Author

a-mr commented Mar 11, 2021

For *.rst files it would be best to assume the code role also but emit a warning (or a hint?) like no default role specified, assuming ".. default-role:: code" — to protect Nim users from unpleasant confusion about why Github/Gitlab don't render it properly.

@timotheecour
Copy link
Member

possible slight improvement: only issue the warning/hint if the rst contains at least one place where this would matter (eg: if user only has things like

``foo``

it wouldn't trigger)

@timotheecour
Copy link
Member

timotheecour commented Mar 17, 2021

@a-mr if you agree, can you add this item to the list above?

  ##
  ## .. warning:: Unsuitable for benchmarking (but still better than `now`),
  ## use `monotimes.getMonoTime` instead or `cpuTime`, depending on use cases.

this should IMO generate a warning similar to #17257 ; users may expect this would render last line as part of the warning but it renders as regular text:
image

it works if i add indentation (even 1 space) to that last line

also, question: how much indentation is "canonical" ?

@a-mr
Copy link
Contributor Author

a-mr commented Mar 17, 2021

yes, we should report a warning, added to the list.

Canonical is certainly 3: all examples in the RST spec are written this way and its syntax diagrams are drawn assuming exactly 3 spaces.

Generally, any indentation is allowed: the text just says "and any subsequent indented text". And rst2html.py allows any number of spaces.

@timotheecour
Copy link
Member

Canonical is certainly 3

then can we produce at least a hint in case user has != 3? otherwise we'll surely end with different styles

@a-mr
Copy link
Contributor Author

a-mr commented Mar 18, 2021

otherwise we'll surely end with different styles

We already did :-) contributing.rst has code-block examples with 1 space of indentation. This repository has variants as 1, 2, 3 and sometimes even 4 and 5.

The more "natural" indentation would be the same as for code in this repository and most Nim projects, which is 2. I think it would be unnecessary complication to learn that in RST directives it "should" be 3. If we really have to proclaim some indentation as canonical then it had better be 2 IMHO.

@timotheecour
Copy link
Member

timotheecour commented Mar 18, 2021

If we really have to proclaim some indentation as canonical then it had better be 2 IMHO.

agreed.

note that there's very little justification left for code-block in nim sources, since runnableExamples is always preferable except for a few remaining edge cases that are fixable (#16993), so instead of fixing indentation for code-block in nim sources we should finish the work of converting remaining ones to runnableExamples.

but for rst files, yes, we should canonicalize on 2 spaces and give a hint/warning when the indent is != 2.

@a-mr
Copy link
Contributor Author

a-mr commented Mar 19, 2021

No, considering the ubiquity of 3 space indentation in RST world, even giving a Hint would be too much.

So let us emit a warning only when indentation is exactly 0 after a directive (even when indentation is less than 0 seems like a perfectly valid use case).

@timotheecour
Copy link
Member

timotheecour commented Mar 24, 2021

can you also add:

`\|` inside a table cell should render as `|` not `\|`
`\|` outside a table cell should keep rendering as `\|` (as it already does since https://github.com/nim-lang/Nim/pull/17315)

(refs future work mentioned in #17315 so it gets referenced here)

a-mr added a commit to a-mr/Nim that referenced this issue Mar 25, 2021
a-mr added a commit to a-mr/Nim that referenced this issue Mar 27, 2021
a-mr added a commit to a-mr/Nim that referenced this issue Apr 7, 2021
Araq pushed a commit that referenced this issue Apr 8, 2021
* further progress on rst roles & dir-s (fix #17646)

* fix documents according to the messages

* fix bug 17 from #17340
@timotheecour
Copy link
Member

timotheecour commented Jun 17, 2021

@a-mr is this a bug, can we improve it or we have to use <> syntax when a link contains parens, eg:

proc main*()=
  ##[
  # these don't work:
  https://en.wikipedia.org/wiki/APL_(programming_language)

  [foo](https://en.wikipedia.org/wiki/APL_(programming_language))
  
  [foo](https://en.wikipedia.org/wiki/APL_\(programming_language\))
  
  [foo](https://en.wikipedia.org/wiki/APL_\(programming_language\))
  
  # this works:  
  `bar <https://en.wikipedia.org/wiki/APL_(programming_language)>`_
  ]##

main()

note: these work fine in gitub markdown:

https://en.wikipedia.org/wiki/APL_(programming_language)

foo

@a-mr
Copy link
Contributor Author

a-mr commented Jun 17, 2021

@timotheecour Yes, according to CommonMark that should be allowed. Added as bug 24.

a-mr added a commit to a-mr/Nim that referenced this issue Jun 25, 2021
and a leftover bug: priority of option list inside definition list
Araq pushed a commit that referenced this issue Jun 26, 2021
and a leftover bug: priority of option list inside definition list
sthagen added a commit to sthagen/nim-lang-Nim that referenced this issue Jun 26, 2021
a-mr added a commit to a-mr/Nim that referenced this issue Jan 22, 2022
Fixes silent disappearance of Markdown (pseudo-)link when it's detected as
unsafe protocol. Now it will be converted to plain text in spirit of
[the specification](https://spec.commonmark.org/0.30/#links).
For that sake the check for protocol is added to rst.nim also.
a-mr added a commit to a-mr/Nim that referenced this issue Jan 31, 2022
Fixes silent disappearance of Markdown (pseudo-)link when it's detected as
unsafe protocol. Now it will be converted to plain text in spirit of
[the specification](https://spec.commonmark.org/0.30/#links).
For that sake the check for protocol is added to rst.nim also.
Varriount pushed a commit that referenced this issue Feb 7, 2022
Fixes silent disappearance of Markdown (pseudo-)link when it's detected as
unsafe protocol. Now it will be converted to plain text in spirit of
[the specification](https://spec.commonmark.org/0.30/#links).
For that sake the check for protocol is added to rst.nim also.
PMunch pushed a commit to PMunch/Nim that referenced this issue Mar 28, 2022
…lang#17659)

* further progress on rst roles & dir-s (fix nim-lang#17646)

* fix documents according to the messages

* fix bug 17 from nim-lang#17340
PMunch pushed a commit to PMunch/Nim that referenced this issue Mar 28, 2022
PMunch pushed a commit to PMunch/Nim that referenced this issue Mar 28, 2022
and a leftover bug: priority of option list inside definition list
PMunch pushed a commit to PMunch/Nim that referenced this issue Mar 28, 2022
Fixes silent disappearance of Markdown (pseudo-)link when it's detected as
unsafe protocol. Now it will be converted to plain text in spirit of
[the specification](https://spec.commonmark.org/0.30/#links).
For that sake the check for protocol is added to rst.nim also.
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

No branches or pull requests

2 participants