black icon indicating copy to clipboard operation
black copied to clipboard

Added test for the Khmer language

Open KaiSforza opened this issue 1 year ago • 8 comments

Description

It looks like this was an issue with the _width_table.py file being out of date in some way.

Closes #4235.

Checklist - did you ...

  • [x] Add an entry in CHANGES.md if necessary?
  • [x] Add / update tests if necessary?
  • [x] Add new / update outdated documentation?

KaiSforza avatar Feb 26 '24 23:02 KaiSforza

@rtb-zla-karma I hope this fixes your issue! Let me know if there's anything else to test here!

(I apologize for the clumsy copy/paste I added in there, I just needed to get a line that was definitely over the limit.)

KaiSforza avatar Feb 26 '24 23:02 KaiSforza

diff-shades results comparing this PR (46e0a240546324ba1654c57af4fa0e9ed09fda41) to main (899002399a26348198612503ce6ca2fc298551a6). The full diff is available in the logs under the "Generate HTML diff report" step.

╭─────────────────────── Summary ────────────────────────╮
│ 1 projects & 2 files changed / 6 changes [+4/-2]       │
│                                                        │
│ ... out of 2 537 182 lines, 12 198 files & 22 projects │
╰────────────────────────────────────────────────────────╯

Differences found.

What is this? | Workflow run | diff-shades documentation

github-actions[bot] avatar Feb 26 '24 23:02 github-actions[bot]

Hi @KaiSforza .

It works for me in my project. black with your changes doesn't report anything where the official/released does.

hwlodarczyk-rtbh avatar Feb 27 '24 15:02 hwlodarczyk-rtbh

This would have to go in the preview style since it changes existing formatting. Not sure the best way to go about it though, since it looks like it'd be a lot of duplicated code. It might be better to just remember to rerun the script before each January release

cobaltt7 avatar Feb 27 '24 16:02 cobaltt7

@RedGuy12 I'll see what I can do. Thanks for the input.

Also, this is a bit of a weird bug, since in #3445 it seems like the _width_table.py file was actually generated with a different script than was committed. Originally the script had this if statement:

if width == 1:
    continue

but the final committed version had a different statement:

if width <= 1:
    continue

This is what is really causing the issue. If I hop into the squashed commit (ef6e079) and run the make_width_table.py script I get the same exact diff as this commit for the width table.

$ git status        
HEAD detached at ef6e079
nothing to commit, working tree clean
$ python ./scripts/make_width_table.py
$ git diff --stat
 src/black/_width_table.py | 364 +---------------------------------------------
 1 file changed, 7 insertions(+), 357 deletions(-)
$ git show --oneline --stat kai/uniwidth
46e0a24 (kaictl/kai/uniwidth, kai/uniwidth) Added test for the Khmer language
 CHANGES.md                                         |   2 +
 src/black/_width_table.py                          | 364 +--------------------
 .../preview_long_strings__east_asian_width.py      |  31 ++
 3 files changed, 40 insertions(+), 357 deletions(-)

To me, this kind of says "it's a bug" and points to not putting it in Preview. From what I can see based on the tests, this shouldn't change anything but a few language strings that are already causing issues in black right now.

(check the commits in #3445 and note that the final commit doesn't change the _width_table.py file, but does change the make_width_table.py)

KaiSforza avatar Feb 27 '24 21:02 KaiSforza

I'm drafting this to do a full rewrite of the unicode character handling. Will probably just push into this same branch to fix it.

KaiSforza avatar Feb 28 '24 18:02 KaiSforza

To me, this kind of says "it's a bug" and points to not putting it in Preview. From what I can see based on the tests, this shouldn't change anything but a few language strings that are already causing issues in black right now.

Unfortunately, Black promises that formatting generally won't change throughout the calendar year, and this change wouldn't be an exception. See the Stability Policy for more details.

cobaltt7 avatar Feb 28 '24 23:02 cobaltt7

That's fine, I'm honestly not sure what the best solution is for languages with ambiguous or non-standardized typesetting.

KaiSforza avatar Feb 28 '24 23:02 KaiSforza