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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Unicode Normalization to speech and braille #16521
Add Unicode Normalization to speech and braille #16521
Changes from 9 commits
0291159
e7581b1
32cffd1
83933da
f41256e
c9fadee
ebde9ef
df5f03d
f6caff8
3d51c24
8d0f95e
161b259
e33c4d6
b8fff11
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Isn't this a tuple as is?
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 you are right. I personally notice that I prefer the more explicit syntax because only brackets can also produce a generator if there is a list comprehension between the brackets. But as far as I'm concerned, the final judgment is up to you.
@coderabbitai what do you think about the suggestion made here?
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.
The
_calculateOffsets
method correctly calculates offsets between the original and normalized strings. Consider adding comments to explain the logic for better readability.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 agree that
_calculateOffsets
needs more commenting. @coderabbitai Could you please generate a suggestion with commenting?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.
@seanbudd, here's the
_calculateOffsets
method with added comments to enhance readability and understanding:This version includes detailed comments that explain each part of the method, making it easier to understand how the offsets are calculated between the original and normalized strings.
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.
@LeonarddeR - can you add comments? When that's done and #16521 (comment) is addressed I think this is ready
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.
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.
@coderabbitai I have just reworded some of your comments in a new commit. DO you think its appropriate now?
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.
@LeonarddeR, I have reviewed the reworded comments in the
_calculateOffsets
method. The comments now provide clear explanations for each part of the method, enhancing readability and understanding. The logic and functionality of the method are well-documented, making it easier for future developers to follow the code.The comments are appropriate and effectively describe the purpose and process of each section of the method. Great job on the rewording!
If there are any further adjustments or additional comments needed, feel free to let me know.
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 4735
New Learnings Added