GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-74902: add unicode grapheme cluster break algorithm by Vermeille · Pull Request #2673 · python/cpython

Conversation

@Vermeille

@Vermeille Vermeille commented Jul 11, 2017 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

I have added GraphemeBreakProperty to UnicodeData.
An automaton to compute the rules for breaking grapheme clusters according to TR29 is included. It passes all the tests provided in GraphemeBreakTests.txt.

https://bugs.python.org/issue30717

  • Issue: Add unicode grapheme cluster break algorithm #74902

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
rsalmei, arhadthedev, cceyda, and woile reacted with hooray emoji
All reactions
  • 🎉 4 reactions
@Vermeille
add unicodedata.grapheme_cluster_break()
b79f969
@the-knights-who-say-ni the-knights-who-say-ni added the CLA not signed label Jul 11, 2017
@the-knights-who-say-ni

the-knights-who-say-ni commented Jul 11, 2017

Copy link
Copy Markdown

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 might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Vermeille Vermeille changed the title WIP: add grapheme cluster break algorithm bpo-30717: WIP: add grapheme cluster break algorithm Jul 11, 2017
@Vermeille
generate unicodedata.c.h with clinic
c9a4211
@the-knights-who-say-ni the-knights-who-say-ni added CLA signed and removed CLA not signed labels Jul 13, 2017
@Vermeille
patchcheck
7f56b78
@Vermeille
Vermeille force-pushed the grapheme_cluster_break branch from 0f82f82 to 62fd6e0 Compare July 13, 2017 23:29
@Vermeille Vermeille changed the title bpo-30717: WIP: add grapheme cluster break algorithm bpo-30717: add grapheme cluster break algorithm Jul 14, 2017
@Vermeille Vermeille changed the title bpo-30717: add grapheme cluster break algorithm bpo-30717: add unicode grapheme cluster break algorithm Jul 14, 2017
@Vermeille
add the grapheme cluster break automaton
a47de54
@Vermeille
Vermeille force-pushed the grapheme_cluster_break branch from 62fd6e0 to a47de54 Compare July 14, 2017 03:21
@Vermeille
add my name to Misc/ACKS
c152171
@Vermeille

Vermeille commented Aug 2, 2017

Copy link
Copy Markdown
Contributor Author

Hello? Someone here?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka
serhiy-storchaka reviewed Aug 3, 2017
View reviewed changes
Comment thread Modules/unicodedata.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/unicodedata.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/grapheme_cluster_break_automaton.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Tools/unicode/makeunicodedata.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka serhiy-storchaka added the type-feature A feature request or enhancement label Aug 3, 2017
@Vermeille
code review fixes
b103be7
methane
methane requested changes Aug 3, 2017
View reviewed changes
Comment thread Modules/unicodedata.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/unicodedata.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/unicodedata.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@Vermeille
rename break_graphemes to iter_graphemes
c9848e2
serhiy-storchaka
serhiy-storchaka reviewed Aug 3, 2017
View reviewed changes
Comment thread Modules/unicodedata.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/unicodedata.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/unicodedata.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/unicodedata.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/unicodedata.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/unicodedata.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Modules/unicodedata.c Outdated
0, /*tp_setattro*/
0, /*tp_as_buffer*/
Py_TPFLAGS_DEFAULT,
"Internal grapheme cluster iterator object.", /* tp_doc */

@serhiy-storchaka serhiy-storchaka Aug 3, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I think the words "internal" and "object" are redundant.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@Vermeille Vermeille Aug 3, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

"Internal", "iterator" and "object" are all redundant. "Grapheme cluster iterator" seems just right. What do you think?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Modules/unicodedata.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Vermeille added 2 commits August 3, 2017 18:23
@Vermeille
make GraphemeClusterIterator a GC type
2dee91e
@Vermeille
allow iterating only over a range of indices
a5b3c10
@Vermeille

Vermeille commented Jan 11, 2018

Copy link
Copy Markdown
Contributor Author

Sorry for the long wait.

Are we good concerning the changes? Anything to add?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@brettcannon

brettcannon commented Feb 2, 2018

Copy link
Copy Markdown
Member

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, I have made the requested changes; please review again. That will trigger a bot to flag this pull request as ready for a follow-up review.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@brettcannon brettcannon added the awaiting changes label Feb 2, 2018
@csabella

csabella commented May 23, 2020

Copy link
Copy Markdown
Contributor

@Vermeille, please take a look at the most recent comments on the bug tracker for this issue. It looks like the suggested path forward is different than the solution you proposed here. Thanks!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions

github-actions Bot commented Feb 20, 2022

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Feb 20, 2022
@ezio-melotti ezio-melotti removed the CLA signed label Jul 13, 2022
@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Jul 28, 2022
@github-actions

github-actions Bot commented Aug 29, 2022

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Aug 29, 2022
@arhadthedev arhadthedev changed the title bpo-30717: add unicode grapheme cluster break algorithm gh-74902: add unicode grapheme cluster break algorithm Feb 14, 2023
@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Feb 15, 2023
@github-actions

github-actions Bot commented Mar 18, 2023

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Mar 18, 2023
@serhiy-storchaka serhiy-storchaka mentioned this pull request Dec 11, 2025
Add functions to get the width in columns of a character #56777
Open
@Vermeille Vermeille mannequin mentioned this pull request Dec 11, 2025
Add unicode grapheme cluster break algorithm #74902
Closed
@serhiy-storchaka
Merge branch 'main' into grapheme_cluster_break
ba1e9b7
@bedevere-app

bedevere-app Bot commented Dec 17, 2025

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka
Add some tests.
ae06154
@bedevere-app

bedevere-app Bot commented Dec 17, 2025

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
serhiy-storchaka
serhiy-storchaka reviewed Dec 17, 2025
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I apologize that it took so long to start reviewing this PR seriously.

Now we need this algorithm to calculate the width of text in columns, which is needed to support wide characters in many parts of the stdlib (REPL, tracebacks, etc). So we will add its implementation anyway. If you are busy or have lost interest, I will finish this work myself (keeping your credit), but if you are still interested, I would be happy to work together.

I wonder, what is the source of the state machine table? Did you created it from the original rules or from the table in GraphemeBreakTest.html? Or copied it from other source? I afraid that it is outdated and only supports legacy grapheme clusters. I can fix this, but maybe you already have a ready solution?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Modules/unicodedata.c

self: self
unistr: unicode
start: int = 0

@serhiy-storchaka serhiy-storchaka Dec 17, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It should be Py_ssize_t. Some other variables should be Py_ssize_t, not int.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Modules/unicodedata.c
self: self
unistr: unicode
start: int = 0
end: Py_ssize_t(c_default="PY_SSIZE_T_MAX - 1") = sys.maxsize

@serhiy-storchaka serhiy-storchaka Dec 17, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It should be PY_SSIZE_T_MAX.

Although I am not sure that the end parameter is needed. The user can simply stop iteration at any time.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@serhiy-storchaka
Save Grapheme_Cluster_Break for unassigned code points.
1965ed6
merwok
merwok reviewed Dec 17, 2025
View reviewed changes
Comment thread Lib/test/test_unicodedata.py
@staticmethod
def check_version(testfile):
hdr = testfile.readline()
return unicodedata.unidata_version in hdr

@merwok merwok Dec 17, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

What does the file header look like?

With string contains tests, I worry about things like "8.0" in "18.0" matching wrongly. Could the full line be compared?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@serhiy-storchaka serhiy-storchaka Dec 17, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

# GraphemeBreakTest-17.0.0.txt

We have the same check for normalization tests.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@serhiy-storchaka
Make "Any" the first entry.
164d19b
@bedevere-app

bedevere-app Bot commented Dec 17, 2025

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@serhiy-storchaka
serhiy-storchaka marked this pull request as draft December 17, 2025 22:16
@bedevere-app bedevere-app Bot removed the awaiting changes label Dec 17, 2025
@serhiy-storchaka serhiy-storchaka self-assigned this Dec 17, 2025
@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Dec 18, 2025
@AA-Turner

AA-Turner commented Dec 22, 2025

Copy link
Copy Markdown
Member

Closing in favour of #143076.

A
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@AA-Turner AA-Turner closed this Dec 22, 2025
@github-project-automation github-project-automation Bot moved this from Todo to Done in CPython pull requests to have finally merged Dec 22, 2025
@serhiy-storchaka serhiy-storchaka removed their assignment Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@merwok merwok merwok left review comments
@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments
@methane methane methane requested changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

type-feature A feature request or enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

9 participants

@Vermeille @the-knights-who-say-ni @brettcannon @csabella @AA-Turner @methane @merwok @serhiy-storchaka @ezio-melotti
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.