◐ Shell
reader mode source ↗
Skip to content

bpo-24665: Add CJK support in textwrap by default.#5649

Closed
JulienPalard wants to merge 1 commit into
python:masterfrom
JulienPalard:textwrap-cjk
Closed

bpo-24665: Add CJK support in textwrap by default.#5649
JulienPalard wants to merge 1 commit into
python:masterfrom
JulienPalard:textwrap-cjk

Conversation

@JulienPalard

@JulienPalard JulienPalard commented Feb 13, 2018 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

@fgallaire

fgallaire commented Feb 14, 2018

Copy link
Copy Markdown

Don't see my author credit

@fgallaire

fgallaire commented Feb 15, 2018

Copy link
Copy Markdown

And you miss the if self.width <= 0: bug fixed in #89

@JulienPalard JulienPalard force-pushed the textwrap-cjk branch 2 times, most recently from 45fd84d to 4623375 Compare March 6, 2018 22:42
@JulienPalard @fgallaire
bpo-24665: Add CJK support in textwrap by default. …
57b2882 
Co-authored-by: Florent Gallaire <fgallaire@gmail.com>
@JulienPalard

JulienPalard commented Mar 6, 2018

Copy link
Copy Markdown
Member Author

And you miss the if self.width <= 0: bug fixed in #89

You're right! And trying to split a wide character yield to an infinite loop.

Don't see my author credit

Gladly fixed and co-authored you.

@fgallaire

fgallaire commented Mar 6, 2018

Copy link
Copy Markdown

Thanks @JulienPalard, I'm so happy ! I had almost lost hope to see this issue fixed.

@JulienPalard JulienPalard requested a review from vstinner March 28, 2018 21:16
terryjreedy
terryjreedy requested changes Jul 8, 2018
View reviewed changes

@terryjreedy terryjreedy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

The change I request is that this be closed because it is conceptually wrong. Textwrap works in terms of abstract 'characters' (codepoint), not physical units. I will explain this on the issue.

Aside from that, 2 is the wrong number to add, as 'double-width' characters are not actually twice as wide as fixed-pitch Ascii chars of the same height. See the issue for this as well.

@bedevere-bot

bedevere-bot commented Jul 8, 2018

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@methane methane closed this Jul 11, 2018
@JulienPalard JulienPalard deleted the textwrap-cjk branch June 16, 2019 14:07
@fgallaire fgallaire mannequin mentioned this pull request Apr 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@terryjreedy terryjreedy terryjreedy requested changes

@larryhastings larryhastings Awaiting requested review from larryhastings

@vstinner vstinner

+1 more reviewer

@fgallaire fgallaire fgallaire left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

awaiting changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@JulienPalard @fgallaire @bedevere-bot @terryjreedy @methane @the-knights-who-say-ni