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

Error when parsing a directive option with ** #712

Open
arwedus opened this issue Feb 21, 2023 · 5 comments
Open

Error when parsing a directive option with ** #712

arwedus opened this issue Feb 21, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@arwedus
Copy link

arwedus commented Feb 21, 2023

Describe the bug

context
When parsing the following document snippet, I get an error:

The maps component is composed of the following packages:

```{sw-package-import}
:dependency_graph: False
:glob: **
```

I wrote a little sphinx plug-in to provide the "sw-package-import" directive. It takes an option glob ("directives.unchanged") which takes a string, in this case the glob pattern "**", which should be passed directly to the directive.

The following syntax also fails with the same error message:

```{sw-package-import}
---
dependency_graph: False
glob: **
---
```

When I replace the above snippet by this, it works:

The maps component is composed of the following packages:

```{eval-rst}
.. sw-package-import::
   :dependency_graph: False
   :glob: **
```

When I put the string into double quotes in the examples 1 and 2, it also works:

```{sw-package-import}
:dependency_graph: False
:glob: "**"
```

expectation

I expected that the option strings are passed "as-is" to the sphinx/docutils directive parser, i.e. I expected all three styles to have the same result.

bug

Here's the error message I ran into with variant 1 and 2:

..../lib/readme.md:5: ERROR: Directive 'sw-package-import': Invalid options YAML: while scanning an alias
  in "<unicode string>", line 2, column 7:
    glob: **
          ^
expected alphabetic or numeric character, but found '*'
  in "<unicode string>", line 2, column 8:
    glob: **
           ^

:dependency_graph: False
:glob: **

I interpreted the hint about YAML syntax right - if you put the ** in double quotes (which makes sure YAML interprets the argument as a string and not something else), the string "**" is returned as option value, as expected.

problem

This is a problem for us because we usually avoid using the extra "eval-rst" directive around sphinx directives to keep the documents shorter. It also feels kinda inconsistent. If this behavior is intended and as specified, I have missed the hint in the documentation. In this case, I kindly ask you to explain the argument parsing in myst markdown directives in more detail in the user documentation.

I think it's actually okay to put strings like these in double quotes, but the documentation should make clear that even the :option: value syntax differs from restructuredtext.

Reproduce the bug

I did not publish the code of my little "sw-package-import" directive yet, but if you need that to get to the root cause of this error message, contact me directly.

List your environment

myst-parser==0.18.1
sphinx==5.3.0
docutils==0.17.1
sphinx-needs==1.2.2

Python version: 3.8.10

OS: Ubuntu Linux 20.04

@arwedus arwedus added the bug Something isn't working label Feb 21, 2023
@chrisjsewell
Copy link
Member

Heya, you just need to wrap the ** in quotes, to denote it as a string:

```{sw-package-import}
:dependency_graph: False
:glob: "**"
```

Yes its a bit of a pain, but thats just how it works at the moment 😅

@arwedus
Copy link
Author

arwedus commented Feb 21, 2023

@chrisjsewell: True, I just found it out myself after thinking about the (YAML related) error message a bit. Then, I think a hint in the documentation should be added that even for the rst-equivalent field list syntax, (:option: value), ambiguous strings have to be wrapped in double quotes even when they don't have to be wrapped in original rst text.<

/edit: wow your reply was super fast, thank you!

@rowanc1
Copy link
Member

rowanc1 commented Feb 21, 2023

I think @chrisjsewell we were talking a while back about changing these to not be YAML options, and follow RST a bit closer. I quite like that direction, especially with the block-attributes work that is going on, we are moving away from yaml (and its quirks) being the main way to parse the directive options.

@chrisjsewell
Copy link
Member

Yep exactly, really we want to be able to parse the option blocks without anything a complicated as YAML; which was the easiest thing to add initially but is annoying from an integration standpoint.

I'd note:

  1. there is now the https://mdit-py-plugins.readthedocs.io/en/latest/#field-lists plugin, that we could think about using here
  2. I'm following Metadata jgm/djot#35 (comment) with interest, to see what they think about there

For directive options, really all we need is a mapping of str -> str, so e.g.

:a: b

is fine

YAML does give you some additional nice features, for longer strings:

# replace new lines with spaces
a: >
   a really long
   string
# keep new lines
b: |
   a really long
   string

but the rest of the complexity is problematic

The only place where we really need the full JSON capability currently is with code-cell, i.e. mapping notebook cell metadata to something. As noted though, perhaps this should be separate from a directive

@rowanc1
Copy link
Member

rowanc1 commented Feb 21, 2023

One thing we have been considering is making the simple options simple:

```{directive}
:option: is a **string
```

and if you specify it as a YAML block, then it is parsed as YAML:

```{directive}
---
option: >
    this is parsed as YAML, and is all *fancy* and stuff.
---
```

This I think gets you to have all of the usual things that mostly just work (strings, flags, numbers, booleans), and an escape hatch for doing very complex arguments without having to invent a new format. The second way of passing in metadata to a directive would be very opt-in, and only for those who really need it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants