✨ Add the eval-rst directive #226

stephenroller · 2020-08-25T00:42:34Z

Adds support the eval-rst directive, similar to recommonmark's version. Fixes #164.

Examples (which I added unit tests for):

Links:

```{eval-rst}
`MyST Parser <https://myst-parser.readthedocs.io/>`_
 ```

Bold text:

```{eval-rst}
**bold**
 ```

Or as I'll use it in ParlAI. I tested this in my own repo.

```{eval-rst}
.. automodule:: parlai.utils.bpe
   :members:
```

welcome · 2020-08-25T00:42:36Z

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! 🎉

codecov · 2020-08-25T00:45:10Z

Codecov Report

Merging #226 into master will decrease coverage by 0.07%.
The diff coverage is 87.50%.

@@            Coverage Diff             @@
##           master     #226      +/-   ##
==========================================
- Coverage   91.00%   90.92%   -0.08%     
==========================================
  Files          12       12              
  Lines        1301     1323      +22     
==========================================
+ Hits         1184     1203      +19     
- Misses        117      120       +3

Flag	Coverage Δ
#pytests	`90.92% <87.50%> (-0.08%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
myst_parser/mocking.py	`86.40% <72.72%> (-0.69%)`	⬇️
myst_parser/docutils_renderer.py	`93.33% <100.00%> (+0.17%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 32f65d5...409b05d. Read the comment docs.

chrisjsewell · 2020-08-25T01:11:04Z

Thanks @stephenroller! a few points:

Can you make it {eval_rst}, so it is consistent with other directive blocks (I guess this will require putting it above the parsing of other directives).

The env is parsed, so that should work with resolving references correctly 👍, if you could add a test for this in tests/test_sphinx, in a new "source" folder (let me know if you need any help with that); testing with referencing from outside -> inside and inside -> outside.

I think error reporting won't quite work correctly at present though. I think this could work, which is along the lines of what I do in myst_parser/mocking.py:

Set the newdoc["source"] = document["source"] before the parse
Set newdoc.reporter = document.reporter
It would also be ideal to have the correct line numbers reported; during the rST parse and after (for things like unknown directives, missing references, etc)
- For during not quite sure just yet, maybe temporarily override the reporter like I do in myst_parser/mocking.py:MockIncludeDirective
- For after you should traverse the tree and update line numbers for all nodes, something like:

for node in newdoc.traverse():
  if node.line:
    node.line += token.map[0]

chrisjsewell

see comment

chrisjsewell · 2020-08-25T01:29:11Z

Also in https://github.com/chrisjsewell/docutils/blob/8adab0660b2097b4f3c32cef7e5ff4cb3c72b084/docutils/docutils/parsers/rst/__init__.py#L193-L194, I note the rST parser removes the default role.
We would want to reverse that, or perhaps not directly use Parser.parse and instead create a subclass, to remove side effects:

class CustomParser(Parser):
    def parse(self, inputstring, document, reporter):
        """Parse `inputstring` and populate `document`, a document tree."""

        self.inputstring = inputstring
        self.document = document
        self.reporter = reporter

        self.statemachine = states.RSTStateMachine(
              state_classes=self.state_classes,
              initial_state=self.initial_state,
              debug=document.reporter.debug_flag)
        inputlines = docutils.statemachine.string2lines(
              inputstring, tab_width=document.settings.tab_width,
              convert_whitespace=True)
        self.statemachine.run(inputlines, document, inliner=self.inliner)

chrisjsewell · 2020-08-25T01:34:50Z

We might even able to "spoof" the line number reporting at this point, like

class CustomParser(Parser):
    def parse(self, inputstring, document, reporter, directive_line):
        """Parse `inputstring` and populate `document`, a document tree."""

        self.inputstring = inputstring
        self.document = document
        self.reporter = reporter

        self.statemachine = states.RSTStateMachine(
              state_classes=self.state_classes,
              initial_state=self.initial_state,
              debug=document.reporter.debug_flag)
        inputlines = docutils.statemachine.string2lines(
              inputstring, tab_width=document.settings.tab_width,
              convert_whitespace=True)
        inputlines = ["" for _ in range(directive_line)] + inputlines
        self.statemachine.run(inputlines, document, inliner=self.inliner, input_offset=directive_line)

chrisjsewell · 2020-08-25T01:39:05Z

Oh and also maybe set the document language: newdoc.settings.language_code = document.settings.language_code

chrisjsewell · 2020-08-25T01:41:20Z

So just a few things for you to do there 😆
Ps yeh I saw you'd started using it in parlai, which is awesome 😁

stephenroller · 2020-08-25T02:25:18Z

Setting source and doing the line adjustments as suggested works.

I think I've got references working but the sphinx tests don't build locally for me. Pushing to see what CI says.

Not sure I understand what that side effect you pointed out is. Could you clarify? (It looks quite innocuous...)

chrisjsewell · 2020-08-25T02:29:54Z

thanks

Not sure I understand what that side effect you pointed out is.

So in RST the syntax for literal strings is with two `, i.e. ``hallo``.
Some people use `hallo`, like in Markdown, that parses hallo to a default role, which sphinx sets by default as :literal:`hallo` , but can actually be set as anything: https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-default_role

chrisjsewell · 2020-08-25T02:33:26Z

I think I've got references working but the sphinx tests don't build locally for me.

You need to update the pytest-regressions file, try running pytest --force-regen, this will fail but update the file, then run again normally

(or tox -e py37-sphinx3 -- --force-regen)

mmcky · 2020-08-25T02:45:04Z

thanks @stephenroller for this PR -- this will be useful.

This is more for discussion than a suggested change. Is there any reason we couldn't just name the directive rst (rather than eval_rst). One argument against this would be possible confusion with just syntax highlighting of rst text.

Directive:

```{rst}
<rst>
```

vs.

Syntax Highlighting:

```rst
<rst>
```

Also, many of the core docutils provided directives seem to adopt a name style that uses - rather than _ (i.e. code-block) -- so perhaps we should use eval-rst? Sphinx seems to use no separators such as literalinclude.

@choldgraf @chrisjsewell should we adopt a naming standard / convention for directives for consistency and add it to CONTRIBUTING.md?

chrisjsewell · 2020-08-25T02:46:43Z

so perhaps we should use eval-rst?

Yeh thats probably better 👍

stephenroller · 2020-08-25T03:32:03Z

Alright I think I addressed all the comments. From the coverage, you can see the empty role block is never being hit; not sure why that is.

chrisjsewell · 2020-08-25T03:39:22Z

Alright I think I addressed all the comments. From the coverage, you can see the empty role block is never being hit; not sure why that is.

Ok ta, so basically the final thing are what is the warnings you get for:

some external

lines

```{eval-rst}
some internal

lines

.. unknown:: some text

.. automodule:: idontexist
   :members:
```

stephenroller · 2020-08-25T04:02:14Z

After the commit I just added:

$ rm -rf _build && sphinx-build . _build
Running Sphinx v2.2.0
making output directory... done
myst v0.12.1: MdParserConfig(renderer='sphinx', commonmark_only=False, dmath_enable=True, dmath_allow_labels=True, dmath_allow_space=True, dmath_allow_digits=True, amsmath_enable=False, override_mathjax=True, admonition_enable=False, disable_syntax=[], html_img_enable=False, url_schemes=None)
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 2 source files that are out of date
updating environment: [new config] 2 added, 0 changed, 0 removed
reading sources... [100%] index
/private/home/roller/working/forks/MyST-Parser/tests/test_sphinx/sourcedirs/references/bad.md:10: WARNING: Unknown directive type "unknown".

.. unknown:: some text
/private/home/roller/working/forks/MyST-Parser/tests/test_sphinx/sourcedirs/references/bad.md:12: WARNING: Unknown directive type "automodule".

.. automodule:: idontexist
   :members:
looking for now-outdated files... none found
pickling environment... done
checking consistency... /private/home/roller/working/forks/MyST-Parser/tests/test_sphinx/sourcedirs/references/bad.md: WARNING: document isn't included in any toctree
done
preparing documents... done
writing output... [100%] index
generating indices...  genindexdone
writing additional pages...  searchdone
copying static files... ... done
copying extra files... done
dumping search index in English (code: en)... done
dumping object inventory... done
build succeeded, 3 warnings.

The HTML pages are in _build.
devfair0237 references evalrst $ nl -ba bad.md
     1  some external
     2
     3  lines
     4
     5  ```{eval-rst}
     6  some internal
     7
     8  lines
     9
    10  .. unknown:: some text
    11
    12  .. automodule:: idontexist
    13     :members:
    14  ```

chrisjsewell · 2020-08-25T04:09:25Z

thats the one 😄

so you can just add this to the bottom of tests/test_renderers/fixtures/reporter_warnings.md:

Warnings in eval-rst
.
some external

lines

```{eval-rst}
some internal

lines

.. unknown:: some text

:unknown:`a`
```
.
source/path:10: (ERROR/3) Unknown directive type "unknown".

.. unknown:: some text

source/path:12: (ERROR/3) Unknown interpreted text role "unknown".
.

chrisjsewell · 2020-08-25T04:21:57Z

Sweet!

If you don't mind, I will just push to this branch, to document it.

chrisjsewell · 2020-08-25T08:41:52Z

Alright! See:

I also removed:

            for node in newdoc.traverse():
                if node.line:
                    # keep line numbers aligned
                    node.line += token.map[0]
                node.source = self.document["source"]

Since, after the addition of pseudosource, it was now double counting.

welcome · 2020-08-25T08:50:07Z

Congrats on your first merged pull request in this project! 🎉

Thank you for contributing, we are very proud of you! ❤️

chrisjsewell · 2020-08-25T09:13:21Z

Released in v0.12.2! https://github.com/executablebooks/MyST-Parser/blob/master/CHANGELOG.md#0122---2020-25-08

jankatins · 2020-08-25T12:26:56Z

https://myst-parser--226.org.readthedocs.build/en/226/using/syntax.html#how-directives-parse-content the "ref from inside in this page does not show up as link. Is that intentional?

stephenroller · 2020-08-25T14:10:15Z

docs/using/syntax.md

+
+  Party time!
+
+A reference from inside: ref:`rst-fun-fish`


Ah damn thanks, so close to perfection 😆

chrisjsewell · 2020-08-25T15:49:23Z

"ref from inside in this page does not show up as link. Is that intentional?

Thanks for the spot, fixed in: https://myst-parser.readthedocs.io/en/latest/using/syntax.html#how-directives-parse-content

stephenroller changed the title ~~Add support for eval_rst~~ [ENH] Add support for eval_rst Aug 25, 2020

✨ ENH: Add support for eval_rst.

47a498c

stephenroller force-pushed the evalrst branch from c12eb99 to 47a498c Compare August 25, 2020 00:47

chrisjsewell requested changes Aug 25, 2020

View reviewed changes

Reviewer comments executablebooks#1.

bbd8b33

Attempt to get references going.

cc74491

Guessing at the correct output.

699ee9c

stephenroller added 5 commits August 24, 2020 23:10

Update the regression.

35f0177

Fix windows bug. Rename to eval-rst.

2148c4d

RST parser which avoids the side effect.

20e1c97

Black.

248dadd

Black again.

14e624a

stephenroller changed the title ~~[ENH] Add support for eval_rst~~ [ENH] Add support for eval-rst directive Aug 25, 2020

stephenroller requested a review from chrisjsewell August 25, 2020 03:33

Bump up the offset for the reporter.

aaf883d

stephenroller added 2 commits August 25, 2020 00:13

Add new warning fixture.

392ab74

Updated warnings based on edited comment.

30c48a3

📚 Add documentation

409b05d

chrisjsewell approved these changes Aug 25, 2020

View reviewed changes

chrisjsewell changed the title ~~[ENH] Add support for eval-rst directive~~ ✨ Add the eval-rst directive Aug 25, 2020

chrisjsewell merged commit 2c1e1f1 into executablebooks:master Aug 25, 2020

stephenroller deleted the evalrst branch August 25, 2020 14:08

stephenroller commented Aug 25, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Add the eval-rst directive #226

✨ Add the eval-rst directive #226

stephenroller commented Aug 25, 2020 •

edited

Loading

welcome bot commented Aug 25, 2020

codecov bot commented Aug 25, 2020 •

edited

Loading

chrisjsewell commented Aug 25, 2020 •

edited

Loading

chrisjsewell left a comment

chrisjsewell commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020 •

edited

Loading

stephenroller commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020 •

edited

Loading

chrisjsewell commented Aug 25, 2020 •

edited

Loading

mmcky commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020

stephenroller commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020 •

edited

Loading

stephenroller commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020 •

edited

Loading

chrisjsewell commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020

welcome bot commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020

jankatins commented Aug 25, 2020

stephenroller Aug 25, 2020

chrisjsewell Aug 25, 2020

chrisjsewell commented Aug 25, 2020

✨ Add the eval-rst directive #226

✨ Add the eval-rst directive #226

Conversation

stephenroller commented Aug 25, 2020 • edited Loading

welcome bot commented Aug 25, 2020

codecov bot commented Aug 25, 2020 • edited Loading

Codecov Report

chrisjsewell commented Aug 25, 2020 • edited Loading

chrisjsewell left a comment

Choose a reason for hiding this comment

chrisjsewell commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020 • edited Loading

stephenroller commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020 • edited Loading

chrisjsewell commented Aug 25, 2020 • edited Loading

mmcky commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020

stephenroller commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020 • edited Loading

stephenroller commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020 • edited Loading

chrisjsewell commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020

welcome bot commented Aug 25, 2020

chrisjsewell commented Aug 25, 2020

jankatins commented Aug 25, 2020

stephenroller Aug 25, 2020

Choose a reason for hiding this comment

chrisjsewell Aug 25, 2020

Choose a reason for hiding this comment

chrisjsewell commented Aug 25, 2020

stephenroller commented Aug 25, 2020 •

edited

Loading

codecov bot commented Aug 25, 2020 •

edited

Loading

chrisjsewell commented Aug 25, 2020 •

edited

Loading

chrisjsewell commented Aug 25, 2020 •

edited

Loading

chrisjsewell commented Aug 25, 2020 •

edited

Loading

chrisjsewell commented Aug 25, 2020 •

edited

Loading

chrisjsewell commented Aug 25, 2020 •

edited

Loading

chrisjsewell commented Aug 25, 2020 •

edited

Loading