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.

Sign up for GitHub

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

Jump to bottom

Replicate directive parsing behaviour of RST parser(?) #55

Closed
choldgraf opened this issue Feb 19, 2020 · 8 comments · Fixed by #59 or #79
Closed

Replicate directive parsing behaviour of RST parser(?) #55

choldgraf opened this issue Feb 19, 2020 · 8 comments · Fixed by #59 or #79
Assignees
@chrisjsewell
Labels
bug Something isn't working
Milestone
Beta Release

Comments

@choldgraf
Copy link
Member

choldgraf commented Feb 19, 2020 •
edited
Loading

I was looking into using this sphinx extension to handle the MyST vs. rST comparison:

https://github.com/djungelorm/sphinx-tabs

It uses a top-level directive .. tabs:: that should take no arguments. However, I found that when I use this directive in MyST with:

```{tabs}
```

I am getting an error that arguments are passed to the tabs directive. The error shows because the directive says it takes zero arguments.

I am wondering if we are passing an empty argument, or something like this?

Here"s a PR that shows off this behavior: #60
@choldgraf choldgraf added the bug Something isn't working label Feb 19, 2020
@chrisjsewell
Copy link
Member

Yeh no problem, I’ll look into it

@chrisjsewell chrisjsewell self-assigned this Feb 19, 2020
chrisjsewell added a commit that referenced this issue Feb 19, 2020
@chrisjsewell
Split role/directive tests to new file and fix #55
bed2c7d
@choldgraf
Copy link
Member Author

choldgraf commented Feb 19, 2020 •
edited
Loading

I have been trying to figure out what"s going on, and the only difference I can find between the MyST and the docutils statemachine seems to be in the parse_directive_arguments method:

    def parse_directive_arguments(directive, arg_text):

If I print a type on each iteration like so:

    def parse_directive_arguments(directive, arg_text):
        print(type(arg_text))

Then I get these outputs:

  • docutils function: <class "docutils.statemachine.StringList">
  • MyST function: <class "str">

No idea if that has something to do with this error, but I"m just trying to figure out where behavior is different between the two packages

@chrisjsewell
Copy link
Member

Don"t worry I"ve already taken care of it in https://github.com/ExecutableBookProject/myst_parser/tree/restructure-testing, PR to come soon

@chrisjsewell
Copy link
Member

It was because the CodeFence regex was including the line break character as part of the argument string.

@chrisjsewell
Copy link
Member

This PR will also handle the directive option indentation #54

@chrisjsewell chrisjsewell mentioned this issue Feb 19, 2020
Merged
chrisjsewell added a commit that referenced this issue Feb 19, 2020
@chrisjsewell
Restructure testing and directive block parsing (#59)
1a1cd05 
In this PR I have:

1. Restructured the test folder and added an `AstRenderer` and lower level tests for the source text to Markdown (mistletoe) AST.
2. Added `docs/develop/test_infrastructure.md` to explain the testing inrastructure.
3. Restructured the directive block parsing, to make it more modular/understandable and improved error checking and reporting.

fixes #54, fixes #55, fixes #51
@choldgraf
Copy link
Member Author

Note that this doesn"t seem to be fixed. Now when I run the same code (after a rebase), I get a warning instead of a break, but it"s the same warning (1 argument passed while 0 expected). Gonna re-open this so we don"t lose track of it, in case there is an underlying issue that"s causing this

@choldgraf
Copy link
Member Author

Note - here is an example of code that causes this behavior: #60

@choldgraf choldgraf reopened this Feb 19, 2020
@chrisjsewell chrisjsewell changed the title Arguments are passed to directives even if no arguments are specified Replicate directive parsing behaviour of RST parser(?) Feb 20, 2020
@chrisjsewell
Copy link
Member

chrisjsewell commented Feb 20, 2020 •
edited
Loading

To clarify this issue: the docutils parser here
does not rigidly specify the structure of the directive block:

.. <name>:: <arguments>
    <option block>

   <body>

Instead, if no required/optional arguments are specified (by the directive class); the <option block> can start at <arguments> or, if no required/optional arguments are specified and no options are specified; the <body> block can start at <arguments>. Examples:

.. content:: :depth: 2
.. note:: This is body text

@chrisjsewell chrisjsewell mentioned this issue Feb 22, 2020
Closed
@chrisjsewell chrisjsewell added this to the Beta Release milestone Feb 24, 2020
@chrisjsewell chrisjsewell mentioned this issue Feb 24, 2020
Merged
chrisjsewell added a commit that referenced this issue Feb 24, 2020
@chrisjsewell
Improve directive block parsing (#79)
a171075 
* Move directive parsing code to separate module
* Allow content to start on argument block (if no arguments allowed)
* Update syntax.md

fixes #55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chrisjsewell chrisjsewell

Labels
bug Something isn't working
Projects
None yet
Milestone
Beta Release
2 participants
@choldgraf @chrisjsewell