bpo-24665: double-width CJK chars support for textwrap#89
Conversation
* Add ckj option flag, default to False * Add cjkwide(), cjklen() and cjkslices() utilities
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow these steps to help rectify the issue:
Thanks again to your contribution and we look forward to looking at it! |
Sorry, something went wrong.
|
I now have listed my GitHub username at bpo and i had already signed the CLA. |
Sorry, something went wrong.
e52db22 to
5c4b2af
Compare
February 14, 2017 09:26
vstinner
left a comment
There was a problem hiding this comment.
You need to document the name option in Doc/library/textwrap.rst, don't forget ".. versionchanged:: 3.7" markup.
Sorry, something went wrong.
|
By email I proposed a different design. Make existing TextWrapper extensible: add text_width() and truncate() methods. Then add a new TextWrapperCJK which overrides these methods. It would allow to reuse the code for other cases than just CJK. |
Sorry, something went wrong.
|
Your change breaks Python build: Python requires optparse to compile modules like unicodedata, optparse imports textwrap which now always requires unicodedata. Using two different classes, it would be simpler to only import unicodedata when the TextWrapperCJK class is instanciated, "on demand", and so fix the bootstrap issue. |
Sorry, something went wrong.
|
What about a depreciation warning to inform that cjk default will switch to |
Sorry, something went wrong.
|
optparse is deprecated since Python 3.2 so it should not drive this work, but a port to argparse will not solve this problem because of the same cycling dependency. |
Sorry, something went wrong.
Codecov Report
@@ Coverage Diff @@
## master #89 +/- ##
=========================================
Coverage ? 82.38%
=========================================
Files ? 1428
Lines ? 351193
Branches ? 0
=========================================
Hits ? 289333
Misses ? 61860
Partials ? 0Continue to review full report at Codecov.
|
Sorry, something went wrong.
methane
left a comment
There was a problem hiding this comment.
While I agree supporting east_asian_wdith is preferable, I don't like current API.
It's OK for third party library. But for standard library, I prefer more generic API and implementation.
While "practical beats purity", I want glibc's wcwidth for UTF-8 at least. (bonus: option like ambiwidth=double in vim)
Sorry, something went wrong.
Disagreed. I guess text editors and terminals are top two applications in textwrap. I known little about text editors. In terminals, things are quite interesting. Different terminals use different versions of
Different Unicode versions can lead to different results for In CPython, handling multiple Unicode versions sounds impractical. IMO chasing the latest Unicode 9.0 is a good idea. If a terminal emulator is not compatible with Unicode 9.0, it should be fixed. [1] https://sourceware.org/bugzilla/show_bug.cgi?id=20313 |
Sorry, something went wrong.
|
Also, I would recommend rename cjk_foobar stuffs to unicode_foobar. CJK characters occupy a large portion on Unicode tables but not the whole. |
Sorry, something went wrong.
|
@yan12125 sorry, my wording was wrong. (I'm not good at English). BTW, pull request is not place for discussion like this. |
Sorry, something went wrong.
…er shutdown code Fix the implementation to work as documented if a thread dies. Now Stackless kills only tasklets with a nesting level > 0. During interpreter shutdown stackless additionally kills daemon threads, if they execute Python code or switch tasklets. This prevents access violations after clearing the thread states. Add a 1ms sleep for each daemon thread. This way the thread gets a better chance to run. Thanks to Kristján Valur Jónsson for suggesting this improvement. Add a test case for a C assertion violation during thread shutdown. Add some explanatory comments to the shutdown test case. Thanks to Christian Tismer for the sugestion. https://bitbucket.org/stackless-dev/stackless/issues/81 https://bitbucket.org/stackless-dev/stackless/issues/89 (grafted from 2cc41427a347a1ebec4eedc3db06a3664e67f798, 1388a2003957, 6140f5aaca2c and edc9b92ec457)
|
To try and help move older pull requests forward, we are going through and backfilling 'awaiting' labels on pull requests that are lacking the label. Based on the current reviews, the best we can tell in an automated fashion is that a core developer requested changes to be made to this pull request. If/when the requested changes have been made, please leave a comment that says, |
Sorry, something went wrong.
|
I close this because I don't like APIs this PR has and the author seems satisfied by |
Sorry, something went wrong.
Per supervisor [chat L2626] + L2742 + librarian L2737 (close pythia python#87 python#3 ephemera-gap durably). Captures empirical W26 push 84 incident (95c9f9b → 1553c14 fix): - Both cinder_opcode_ids.h + Include/opcode.h use Py_OPCODE_H guard - Include order silently shadows whichever loads second - W26 case: BINARY_OP_ADD_INT undefined → attr_probe HIR regression caught by W21 golden trip-wire Three resolution options enumerated: - (A) rename cinder_opcode_ids.h guard to CINDERX_OPCODE_IDS_H — recommended, eliminates collision class - (B) static_assert in cinder_opcode.h to detect collision at compile — hardens detection without fixing - (C) header-comment only — current state, NOT recommended (chat- ephemera proven insufficient) Related W29 candidate noted: PHX_PRIM_OP_* / PHX_PRIM_UOP_* hard-coded in builder_emit_c.c lines 3727-3746 with no static_assert binding to classloader.h authoritative #defines (per pythia python#89 python#3 re-issue). DEFERRED per supervisor — schedule post-Batch-2 burndown, before any upstream sync touching Include/opcode.h or classloader.h.
edited
LoadingUh oh!
There was an error while loading. Please reload this page.
Copy link Copy MarkdownSorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.