-
Notifications
You must be signed in to change notification settings - Fork 7
Improve sphinx reStructuredText parsing #13
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
Conversation
@@ -624,6 +624,28 @@ def func(): pass | |||
""" | |||
|
|||
|
|||
# https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#info-field-lists | |||
SPHINX_SIGNATURE = """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there code that handles any code blocks before this? Otherwise something like
.. code-block:: python
def foo():
""":param str message_body: blah blah"""
could cause problems. I'll admit it's an edge case that would likely only come up in docstring parsing code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, one other thing that comes to mind! If the programmer put these directives in a random order for some reason, it would be nice if all directives of a certain type were grouped together. Also definitely a bit of an edge case & not required, but there's plenty of less-than-finely crafted code out there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it will not cause problems. Code blocks are properly parsed out at an earlier stage. I added a test case in 1771df2 to demonstrate this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree about the order thing. That would require a proper parser though so we cannot just use regexp here. It is definitely in scope, but I don't have more time this weekend. Would you mind opening an issue so we can track this as a future improvement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resolved with #14
Thanks for the link, and thanks for fixing this! I like the approach you've taken, I've used a similar approach in the past to convert markdown to HTML. |
This should largely fix pappasam/jedi-language-server#172.