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

[WIP] Warn in case news items contains multiple paragraphs. #274

Closed
wants to merge 3 commits into from
Closed

[WIP] Warn in case news items contains multiple paragraphs. #274

wants to merge 3 commits into from

Conversation

JulienPalard
Copy link
Member

@JulienPalard JulienPalard commented Jul 7, 2018

See https://bugs.python.org/issue32523, having multiple paragraphs in
a few news entry lead to inconsistent spacing while rendered in HTML.

Also NEWS entries should stay short, having multiple paragraphs is an
indicator the news entry is too long.

Currently, on cpython master, it yields around 60 warnings.

I'm also trying to fix them in python/cpython#8154

@JulienPalard JulienPalard changed the title Warn in case news items contains multiple paragraphs. [WIP] Warn in case news items contains multiple paragraphs. Jul 7, 2018
JulienPalard added a commit to JulienPalard/cpython that referenced this pull request Jul 7, 2018
Also see: python/core-workflow#274

Having multiple paragraphs in a few news entry lead to inconsistent
spacing while rendered in HTML by mixing "visually compact lists"
(when no entry of the whole list contains multiple paragraphs) and
"sparse lists" (when at least one do).

Also NEWS entries should probably stay short.
@terryjreedy
Copy link
Member

While reviewing #8154, I noticed one case where I left a trailing blank line. This is a simple editing error. If blurb can fix this, it should do so. Otherwise, it should warn 'Trailing blank line(s)' rather than 'Multiple paragraphs'.

@JulienPalard
Copy link
Member Author

Make sense I'll try to make the warning appropriate in this case.

@larryhastings
Copy link
Contributor

I'm pretty sure blurb's aggressive paragraph reflowing will strip all trailing whitespace. See the last few lines of textwrap_body().

@larryhastings
Copy link
Contributor

I think if blurb is going to have a non-fatal error, it should work like this: you edit your blurb, you save and exit. Blurb prints a warning, in this case saying "You have multiple paragraphs! News blurbs should only be a single paragraph. Are you sure? Type 'yes' to use this file, anything else to re-edit." And then it should raw_input() prompt the user. If they type "yes" we continue, otherwise we loop.

How does that sound?

Copy link
Contributor

@larryhastings larryhastings left a comment

Choose a reason for hiding this comment

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

Sorry, I should have put my comments here. Please see comment on the main PR. (Creating this just for scoring purposes for the CPython core dev PRs-reviewed leaderboard.)

@ericvsmith
Copy link
Member

raw_input()? What year is this? :)

See https://bugs.python.org/issue32523, having multiple paragraphs in
a few news entry lead to inconsistent spacing while rendered in HTML.

Also NEWS entries should stay short, having multiple paragraphs is an
indicator the news entry is too long.
@JulienPalard
Copy link
Member Author

I'm currently trying to enhance my PR to have the nice behavior with the

You have multiple paragraphs! News blurbs should only be a single paragraph. Are you sure? Type 'yes' to use this file, anything else to re-edit.

message. But I'm realizing that forbidding paragraphs probably leads to forbidding any formatted kind of things like bullet lists, which anyway cannot be "correctly" represented in the HTML documentation, it goes as a single line like:

bpo-27706: Restore deterministic behavior of random.Random().seed() for string seeds using seeding version 1. Allows sequences of calls to random() to exactly match those obtained in Python 2. Patch by Nofar Schnider.

So I'll probably remove all handling of multi-paragraph things (initially, it was what's causing rendering bugs in the lists), is that OK?

JulienPalard added a commit to JulienPalard/cpython that referenced this pull request May 9, 2019
Also see: python/core-workflow#274

Having multiple paragraphs in a few news entry lead to inconsistent
spacing while rendered in HTML by mixing "visually compact lists"
(when no entry of the whole list contains multiple paragraphs) and
"sparse lists" (when at least one do).

Also NEWS entries should probably stay short.
@JulienPalard
Copy link
Member Author

Oh, a forgotten old PR, hello.

This is more subtle than that, in https://docs.python.org/3.10/whatsnew/changelog.html we have a blurb message like this:

Fix bugs in cleaning up classes and modules in :mod:`unittest`:

* Functions registered with :func:`~unittest.addModuleCleanup` were not called unless the user defines ``tearDownModule()`` in their test module.
* Functions registered with :meth:`~unittest.TestCase.addClassCleanup` were not called if ``tearDownClass`` is set to ``None``.
* Buffering in :class:`~unittest.TestResult` did not work with functions registered with ``addClassCleanup()`` and ``addModuleCleanup()``.
* Errors in functions registered with ``addClassCleanup()`` and ``addModuleCleanup()`` were not handled correctly in buffered and debug modes.
* Errors in ``setUpModule()`` and functions registered with ``addModuleCleanup()`` were reported in wrong order.
* And several lesser bugs.

and yet the sphinx rendering does not use a sparse list because it's a single line and a sub list, so it should be considered valid.

@JulienPalard
Copy link
Member Author

I'm not convinced we should continue this way, some news should probably be shorter but some really need some multi-paragraph formatting, like:

bpo-43244: Remove the compiler and parser functions using struct _mod type, because the public AST C API was removed:

    PyAST_Compile()
    PyAST_CompileEx()
    PyAST_CompileObject()
    PyFuture_FromAST()
    PyFuture_FromASTObject()
    PyParser_ASTFromFile()
    PyParser_ASTFromFileObject()
    PyParser_ASTFromFilename()
    PyParser_ASTFromString()
    PyParser_ASTFromStringObject()

These functions were undocumented and excluded from the limited C API. Patch by Victor Stinner.

or

bpo-43244: Remove the symtable.h header file and the undocumented functions:

    PyST_GetScope()
    PySymtable_Build()
    PySymtable_BuildObject()
    PySymtable_Free()
    Py_SymtableString()
    Py_SymtableStringObject()

The Py_SymtableString() function was part the stable ABI by mistake but it could not be used, because the symtable.h header file was excluded from the limited C API.

The Python symtable module remains available and is unchanged.

Patch by Victor Stinner.

I'm closing this and add a comment on b.p.o.

@JulienPalard JulienPalard deleted the warn-multiple-paragraphs branch September 30, 2021 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants