Skip to content

Docs: Don't wrap the markdown for GitHub releases #6621

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 1 commit into from
Jan 29, 2020
Merged

Docs: Don't wrap the markdown for GitHub releases #6621

merged 1 commit into from
Jan 29, 2020

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Jan 29, 2020

Thanks for submitting a PR, your contribution is really appreciated!

Here is a quick checklist that should be present in PRs.

  • Target the master branch for bug fixes, documentation updates and trivial changes.
  • Target the features branch for new features, improvements, and removals/deprecations.
  • [n/a] Include documentation when adding new features.
  • [n/a] Include new tests or update existing tests when applicable.

Unless your change is trivial or a small documentation fix (e.g., a typo or reword of a small section) please:

  • [n/a?] Create a new changelog file in the changelog folder, with a name like <ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.

    Write sentences in the past or present tense, examples:

    • Improved verbose diff output with sequences.
    • Terminal summary statistics now use multiple colors.

    Also make sure to end the sentence with a ..

  • Add yourself to AUTHORS in alphabetical order.


What happens

Releases on GitHub look like this, with oddly-wrapped text (eg. https://github.com/pytest-dev/pytest/releases/tag/5.3.5):

image

In edit mode, it wraps like this, but of course a lot of that link markdown isn't visible to the end user, and GitHub is respecting the newlines here:

image

https://github.com/pytest-dev/pytest/releases/edit/5.3.5


Why

I understand scripts/publish-gh-release-notes.py is run to convert reStructuredText into Markdown, and post it in the GitHub release. It takes RST from the changelog:

pytest 5.3.5 (2020-01-29)
=========================

Bug Fixes
---------

- `#6517 <https://github.com/pytest-dev/pytest/issues/6517>`_: Fix regression in pytest 5.3.4 causing an INTERNALERROR due to a wrong assertion.

https://github.com/pytest-dev/pytest/blob/master/doc/en/changelog.rst#pytest-535-2020-01-29

Here there's no newlines, in the plain RST, nor the rendered output.

The conversion is made using pypandoc:

def convert_rst_to_md(text):
    return pypandoc.convert_text(text, "md", format="rst")

The fix

pypandoc.convert_text doesn't have any explicit line wrapping arguments, but you can use extra_args to pass to the underlying pandoc tool:

--wrap=auto|none|preserve

Determine how text is wrapped in the output (the source code, not the rendered version). With auto (the default), pandoc will attempt to wrap lines to the column width specified by --columns (default 72). With none, pandoc will not wrap lines at all. With preserve, pandoc will attempt to preserve the wrapping from the source document (that is, where there are nonsemantic newlines in the source, there will be nonsemantic newlines in the output as well).

https://pandoc.org/MANUAL.html

And so:

 def convert_rst_to_md(text):
-    return pypandoc.convert_text(text, "md", format="rst")
+    return pypandoc.convert_text(text, "md", format="rst", extra_args=["--wrap=none"])

Demo

I've not tested this using scripts/publish-gh-release-notes.py and I don't think there's tests for these helper scripts, so here's a short demo:

import pypandoc


def convert_rst_to_md_before(text):
    return pypandoc.convert_text(text, "md", format="rst")


def convert_rst_to_md_after(text):
    return pypandoc.convert_text(text, "md", format="rst", extra_args=["--wrap=none"])


text = """
pytest 5.3.5 (2020-01-29)
=========================

Bug Fixes
---------

- `#6517 <https://github.com/pytest-dev/pytest/issues/6517>`_: Fix regression in pytest 5.3.4 causing an INTERNALERROR due to a wrong assertion.

"""

print("BEFORE")
print(convert_rst_to_md_before(text))
print("AFTER")
print(convert_rst_to_md_after(text))

Output:

BEFORE
pytest 5.3.5 (2020-01-29)
=========================

Bug Fixes
---------

-   [\#6517](https://github.com/pytest-dev/pytest/issues/6517): Fix
    regression in pytest 5.3.4 causing an INTERNALERROR due to a wrong
    assertion.

AFTER
pytest 5.3.5 (2020-01-29)
=========================

Bug Fixes
---------

-   [\#6517](https://github.com/pytest-dev/pytest/issues/6517): Fix regression in pytest 5.3.4 causing an INTERNALERROR due to a wrong assertion.

A preview of this unwrapped text:

image

Copy link
Contributor

@blueyed blueyed left a comment

Choose a reason for hiding this comment

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

Thanks!

@blueyed blueyed added type: enhancement new feature or API change, should be merged into features branch type: infrastructure improvement to development/releases/CI structure labels Jan 29, 2020
@nicoddemus
Copy link
Member

Hi @hugovk,

I've tried with a more complex example and it looks good:

import pypandoc


def convert_rst_to_md_before(text):
    return pypandoc.convert_text(text, "md", format="rst")


def convert_rst_to_md_after(text):
    return pypandoc.convert_text(text, "md", format="rst", extra_args=["--wrap=preserve"])


text = """
pytest 5.3.5 (2020-01-29)
=========================

Bug Fixes
---------

- `#6517 <https://github.com/pytest-dev/pytest/issues/6517>`_: Fix regression in pytest 5.3.4 causing an INTERNALERROR due to a wrong assertion.

- `#6517 <https://github.com/pytest-dev/pytest/issues/6517>`_: Fix regression in pytest 5.3.4
  causing an INTERNALERROR due to a wrong assertion.

  This also checks something else.

  Also see this:

  .. code-block:: python

      def test():
          pass

  Another example.
"""

print("AFTER")
print(convert_rst_to_md_after(text))

"""

print("AFTER")
print(convert_rst_to_md_after(text))
AFTER
pytest 5.3.5 (2020-01-29)
=========================

Bug Fixes
---------

-   [\#6517](https://github.com/pytest-dev/pytest/issues/6517): Fix regression in pytest 5.3.4 causing an INTERNALERROR due to a wrong assertion.

-   [\#6517](https://github.com/pytest-dev/pytest/issues/6517): Fix regression in pytest 5.3.4
    causing an INTERNALERROR due to a wrong assertion.

    This also checks something else.

    Also see this:

    ``` {.python}
    def test():
        pass
    ```

    Another example.

Render: https://gist.github.com/nicoddemus/83a3ef2c6548f481a1d9fe7787be396c

So thanks a lot for looking into this. 👍

@nicoddemus nicoddemus merged commit 757873e into pytest-dev:master Jan 29, 2020
@blueyed
Copy link
Contributor

blueyed commented Jan 29, 2020

@nicoddemus this was waiting for an update AFAICT.

@hugovk hugovk deleted the no-wrap-markdown-for-gh-release branch January 29, 2020 22:46
@hugovk
Copy link
Member Author

hugovk commented Jan 29, 2020

Sorry, I forgot to push that last change. I can do it tomorrow or it's a simple thing if you'd like to do it. Thanks!

@hugovk hugovk restored the no-wrap-markdown-for-gh-release branch January 29, 2020 22:48
@blueyed
Copy link
Contributor

blueyed commented Jan 29, 2020

@hugovk doesn't matter that much/enough I guess.
Thanks again though.. :)

@nicoddemus
Copy link
Member

@nicoddemus this was waiting for an update AFAICT.

Ouch my bad! 🤦‍♂

Will open a new PR.

nicoddemus added a commit to nicoddemus/pytest that referenced this pull request Jan 29, 2020
@nicoddemus
Copy link
Member

Done in #6624.

@hugovk hugovk deleted the no-wrap-markdown-for-gh-release branch January 30, 2020 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement new feature or API change, should be merged into features branch type: infrastructure improvement to development/releases/CI structure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants