Skip to content

Leave trailing line break in reST directives #385

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

Merged
merged 6 commits into from
Feb 3, 2022

Conversation

howamith
Copy link
Contributor

@howamith howamith commented Feb 1, 2022

Closes #384.

Feels a little clunky adding a trailing line break when processing reST directives, to then have to remove it again for reST directives in code blocks, but I couldn't see any other way. Figured I'd submit a PR anyway for feedback and go from there.

Copy link
Member

@kernc kernc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clunky indeed, but unless we have a way to fix the regex:

pdoc/pdoc/html_helpers.py

Lines 325 to 328 in 4aa70de

substitute = partial(re.compile(r'^(?P<indent> *)\.\. ?(\w+)::(?: *(.*))?'
r'((?:\n(?:(?P=indent) +.*| *$))*)', re.MULTILINE).sub,
partial(_ToMarkdown._admonition, module=module,
limit_types=limit_types))

I don't have a really better idea.

As suggested by kernc.

Co-authored-by: kernc <[email protected]>
@howamith
Copy link
Contributor Author

howamith commented Feb 2, 2022

Another possibly way of neatening this up would be to somehow detect the context of where the reST directive is (i.e whether or not it's within a code block) and only add the trailing new-line where appropriate. This context could be detected in, or outside of and passed in, to _ToMarkdown._admonition().

@kernc
Copy link
Member

kernc commented Feb 2, 2022

Another way would be to actually amend the culprit regex. It's posed as:

^(?P<indent> *)\.\. ?(\w+)::(?: *(.*))?
((?:\n(?:(?P=indent) +.*| *$))*)

for its second line to matche any indented paragraphs (separated with a line break). E.g.:

Outside admonition.

.. warning:: Title
   First paragraph.

   Second paragraph.  # Our regex matches this final line break. But if it didn't ...

Outside admonition.

As can be seen here (trailing \n is visible in the "Export Matches" JSON).

I think we could rewrite the regex to not consume the line break unless another indented paragraph follows it ... 🤔

@howamith
Copy link
Contributor Author

howamith commented Feb 3, 2022

Not quite fully isolated to just changing the regex, but I think that's the closest you'll get (though I'm by no means an expert when it comes to regex!).

Let me know what you think - if you're happy with that then I'll squash the redundant commits and let you merge :)

@kernc kernc changed the title Issue 384 include md tables Leave trailing line break in reST directives Feb 3, 2022
@kernc
Copy link
Member

kernc commented Feb 3, 2022

Excellent find of the regex! Many thanks! 🍰

@kernc kernc merged commit 2cce30a into pdoc3:master Feb 3, 2022
@howamith howamith deleted the issue-384-include-md-tables branch February 3, 2022 14:44
kernc pushed a commit to kernc/pdoc that referenced this pull request Jun 22, 2024
* Add to `pdoc.test.Docformats.test_reST_include` to catch issue.

* Add a single line-break at the end of `.. include::`ed files - except for when in a code block.

* Minor change comment

* Revert last 2 commits.

* Don't consume trailing newline from reST directives.

* [] itself a disjunctive list of characters
kernc pushed a commit to johann-petrak/pdoc that referenced this pull request Jun 22, 2024
* Add to `pdoc.test.Docformats.test_reST_include` to catch issue.

* Add a single line-break at the end of `.. include::`ed files - except for when in a code block.

* Minor change comment

* Revert last 2 commits.

* Don't consume trailing newline from reST directives.

* [] itself a disjunctive list of characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Tables in .. include::ed markdown files merge with remaining docstring
2 participants