[issue28822] Fix indices handling in PyUnicode_FindChar

Changes by Donald Stufft : -- pull_requests: +875 ___ Python tracker ___ ___

[issue28822] Fix indices handling in PyUnicode_FindChar

Roundup Robot added the comment: New changeset 2b5e5a3a805e by Martin Panter in branch 'default': Issue #28822: Add susp-ignored entry for NEWS; fix grammar https://hg.python.org/cpython/rev/2b5e5a3a805e -- ___ Python tracker

[issue28822] Fix indices handling in PyUnicode_FindChar

Xiang Zhang added the comment: Thanks Victor and Serhiy! -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue28822] Fix indices handling in PyUnicode_FindChar

Roundup Robot added the comment: New changeset ce6a6cc3765d by Xiang Zhang in branch 'default': Issue #28822: Adjust indices handling of PyUnicode_FindChar(). https://hg.python.org/cpython/rev/ce6a6cc3765d -- nosy: +python-dev ___ Python tracker

[issue28822] Fix indices handling in PyUnicode_FindChar

STINNER Victor added the comment: Ignore my request about special direction values. It's not worth it to writ tests for that. PyUnicode_FindChar-v2.patch LGTM. -- ___ Python tracker

[issue28822] Fix indices handling in PyUnicode_FindChar

Xiang Zhang added the comment: > Remaining question: what is the behaviour for direction=0, direction=100 or > direction=-2? Maybe we can add a few unit tests for strange values of > direction? (Not sure if it's worth it.) It's not documented so I also doubt it. Expect Serhiy's comment.

[issue28822] Fix indices handling in PyUnicode_FindChar

STINNER Victor added the comment: PyUnicode_FindChar-v2.patch LGTM with a minor comment on the review, but I would prefer that Serhiy also reviews it ;-) Remaining question: what is the behaviour for direction=0, direction=100 or direction=-2? Maybe we can add a few unit tests for strange

[issue28822] Fix indices handling in PyUnicode_FindChar

Xiang Zhang added the comment: Thanks for your reviews. :-) v2 updated the test codes. -- Added file: http://bugs.python.org/file45690/PyUnicode_FindChar-v2.patch ___ Python tracker

[issue28822] Fix indices handling in PyUnicode_FindChar

Serhiy Storchaka added the comment: Would be nice to test corner cases: 1. Search UCS2 or UCS4 character with zero lower 8 bits: U+XX00. 2. Search UCS2 or UCS4 character with lower 8 bits that match high bits of string characters. For example search U+0404 in the string that consists of

[issue28822] Fix indices handling in PyUnicode_FindChar

Serhiy Storchaka added the comment: I think it is nice to add tests for C API. Especially if there is no direct mapping between Python and C API ("character in str" don't call PyUnicode_FindChar()). Tests should cover all corner cases, otherwise we can miss bugs. Some C API can be not used in

[issue28822] Fix indices handling in PyUnicode_FindChar

STINNER Victor added the comment: Serhiy: I don't think that it's worth it to add a new function to _testcapi to test PyUnicode_FindChar. The implementation of the function seems simple. At least, I would prefer to only see a few unit tests, not 17 test for this simple function! I mean

[issue28822] Fix indices handling in PyUnicode_FindChar

Xiang Zhang added the comment: Other APIs like PyUnicode_Find and PyUnicode_Count support it. Their docs are almost the same so I think PyUnicode_FindChar does not need to be the special one. After change, its behaviour and implementation are more consistent with other APIs. --

[issue28822] Fix indices handling in PyUnicode_FindChar

STINNER Victor added the comment: PyUnicode_FindChar.patch is a new feature, it cannot be applied to stable branches (py < 3.7). I'm not sure that it's worth it to support negative indexes for end. Why not simply documenting that end must be positive? --

[issue28822] Fix indices handling in PyUnicode_FindChar

New submission from Xiang Zhang: PyUnicode_FindChar declares in the doc it treats its *start* and *end* parameters as str[start:end], same as other APIs like PyUnicode_Find, PyUnicode_Count. But it doesn't allow negative indices like others so violates the doc. -- components: