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

Fix line breaks added after hyphenations by blurb. #7002

Merged
merged 2 commits into from
May 20, 2018

Conversation

serhiy-storchaka
Copy link
Member

No description provided.

@serhiy-storchaka
Copy link
Member Author

@terryjreedy, please take a look at IDLE entries. Seems there are issues with a bullet list.

@terryjreedy
Copy link
Member

I just removed the asterisks and it looks OK to me.

@terryjreedy
Copy link
Member

I gather that when blurb was introduced, the existing 3.5 and 3.6 news entries were split apart into individual files and then reassembled by blurb. In the process, it re-flowed the text, I presume with textwrap, sometimes putting newlines after hyphens.

Why you think this is a problem? It is standard practice in English text composition and sometimes, new hyphens are introduced between syllables. (Textwrap is not sophisticated enough to do that.)

When Sphinx collects the x.y .rst news files into an x.y changelog, it will rewrap again, changing the locations of newlines, but again occasionally putting them after a hyphen, before justifying with whitespace. See https://docs.python.org/3/whatsnew/changelog.html#changelog for example.

The question is whether having newlines follow - in .rst files affects the Sphinx reflow process. Does Sphinx treat line-ending -s as optional sofe -s, eligible to be deleted along with \n? From my cursory reading I did not see any obvious examples of this.

In other words, I think this needed an issue and discussion as to whether to make such a change, and if so, why.

@terryjreedy
Copy link
Member

I gather that when blurb was introduced, the existing 3.5 and 3.6 news entries were split apart into individual files and then reassembled by blurb. In the process, it re-flowed the text, I presume with textwrap, sometimes putting newlines after hyphens.

Why you think this is a problem? It is standard practice in English text composition and sometimes, new hyphens are introduced between syllables. (Textwrap is not sophisticated enough to do that.)

When Sphinx collects the x.y .rst news files into an x.y changelog, it will rewrap again, changing the locations of newlines, but again occasionally putting them after a hyphen, before justifying with whitespace. See https://docs.python.org/3/whatsnew/changelog.html#changelog for example.

The question is whether having newlines follow - in .rst files affects the Sphinx reflow process. Does Sphinx treat line-ending -s as optional sofe -s, eligible to be deleted along with \n? From my cursory reading I did not see any obvious examples of this.

In other words, I think this needed an issue and discussion as to whether to make such a change, and if so, why.

PS. The IDLE entries could stand being changed anyway, but that can be done separately and later. An alternative to deleting the *s would be to convert them to the .rst markup. There may be other entries that need change.

@serhiy-storchaka
Copy link
Member Author

Thank you for fixing IDLE entries Terry.

Yes, the problem is that this affects the Sphinx reflow process. For example, look at the entry edited by you: I think Sphinx- like is incorrect writing in English, and the correct one is Sphinx-like. In any case inserting a space inside a long option (like in -Werror=declaration- after-statement) is not correct.

@serhiy-storchaka serhiy-storchaka merged commit aef639f into python:master May 20, 2018
@miss-islington
Copy link
Contributor

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

@serhiy-storchaka serhiy-storchaka deleted the docs-hyphen-break branch May 20, 2018 23:36
@miss-islington
Copy link
Contributor

Sorry, @serhiy-storchaka, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker aef639f62677f8a342af24e9c19f0503b0d1e36e 3.7

@miss-islington
Copy link
Contributor

Sorry, @serhiy-storchaka, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker aef639f62677f8a342af24e9c19f0503b0d1e36e 3.6

@terryjreedy
Copy link
Member

'Sphinx- like' is definitely a bug. The problem is an incompatibility between the default options of Textwrap for double reflows and a bug in its usage by blurb.

replace_whitespace=True replaces all white spaces, including \n, with ' '.
break_on_hyphens=True allows insertion of \n after '-' as an alternative to replacement of ' ' by '\n'. So reflow 1 inserts \n, reflow 2 replaces \n with '', net result, insertion of ' ', a bug.

To avoid the problem in the future, blurb should pass break_on_hyphens=False. Your patch has the effect of doing this. If you have not manually done the backports yet, you could fix blurb and rebuild the files as an alternative.

@serhiy-storchaka
Copy link
Member Author

This will not help because lines already were broken on hyphens.

@terryjreedy
Copy link
Member

If people, including me, were breaking on hyphens when writing blurbs, they should not at Textwrap is currently written. It seems to me that in text, replace_whitespace should replace '-\n' with '-' rather than '- '.

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request May 22, 2018
Also remove bullet asterisks from IDLE entries..
(cherry picked from commit aef639f)

Co-authored-by: Serhiy Storchaka <[email protected]>
@bedevere-bot
Copy link

GH-7050 is a backport of this pull request to the 3.7 branch.

serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request May 22, 2018
Also remove bullet asterisks from IDLE entries..
(cherry picked from commit aef639f)

Co-authored-by: Serhiy Storchaka <[email protected]>
@bedevere-bot
Copy link

GH-7051 is a backport of this pull request to the 3.6 branch.

serhiy-storchaka added a commit that referenced this pull request May 22, 2018
Also remove bullet asterisks from IDLE entries.
(cherry picked from commit aef639f)
serhiy-storchaka added a commit that referenced this pull request May 22, 2018
Also remove bullet asterisks from IDLE entries.
(cherry picked from commit aef639f)
@serhiy-storchaka serhiy-storchaka removed their assignment Dec 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants