When spelling by line, ensure that only normalized characters report as such #17295
Conversation
|
@coderabbitai review |
WalkthroughThe changes in this pull request primarily enhance the speech processing capabilities of the NVDA application. Key modifications include updates to the Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
source/speech/speech.py (1)
Line range hint
421-499: Recommend Adding Unit Tests for Character NormalizationTo ensure the correctness of the new normalization logic and prevent future regressions, consider adding unit tests that cover edge cases where characters are normalized individually during spelling.
Would you like assistance in generating these unit tests or opening a new GitHub issue to track this task?
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
📒 Files selected for processing (2)
- source/speech/speech.py (4 hunks)
- user_docs/en/changes.md (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
source/speech/speech.py (2)
Pattern
**/*: Focus on code smells, logic errors, edge cases, missing test cases, security flaws and serious issues. Avoid commenting on minor issues such as linting, formatting and style issues. This project uses tabs instead of spaces, do not suggest usage of spaces over tabs. Are there any 'red flags' in this code that might warrant closer investigation from a security standpoint? Explain what makes them suspicious. When providing code suggestions, particularly when requested, ensure GitHub's suggestion format is used, i.e.:suggestion <code changes>
Pattern
**/*.py: _, pgettext, ngettext, and ngettext are defined globally, errors for this being undefined can be ignored.user_docs/en/changes.md (3)
Pattern
**/*: Focus on code smells, logic errors, edge cases, missing test cases, security flaws and serious issues. Avoid commenting on minor issues such as linting, formatting and style issues. This project uses tabs instead of spaces, do not suggest usage of spaces over tabs. Are there any 'red flags' in this code that might warrant closer investigation from a security standpoint? Explain what makes them suspicious. When providing code suggestions, particularly when requested, ensure GitHub's suggestion format is used, i.e.:suggestion <code changes>
Pattern
**/*.md: Focus on grammar, spelling, and punctuation. Also consider readability and clarity of contents. Ensure that changes follow the style of one sentence per line, suggest changes if this is not met.
Pattern
user_docs/en/changes.md: Ensure each change log entry references an issue or pull request number. Change log entries can also include a reference to a GitHub author. Examples of valid change log entries: * Item with sub-items (#123, @username): * sub-item * bar (#342) * Item with. Multiple lines. (#143)
🔇 Additional comments (6)
user_docs/en/changes.md (5)
Line range hint
3-7: Overview of NVDA 2025.1 releaseThis release introduces several new features and improvements, including enhanced support for Microsoft Office applications, new braille functionality, and various bug fixes. It also includes some important changes for developers.
Line range hint
9-15: Notable new features
- Support for charts in Microsoft Word and PowerPoint
- Automatic reporting of new text output in various applications
- New braille translation tables and improved braille input support
- Experimental support for Google Sheets with Braille mode
Line range hint
17-24: Significant changes and improvements
- Updated components: liblouis braille translator, eSpeak-NG, Unicode CLDR
- Improvements to Microsoft Office support, including Excel and Word
- Enhanced support for braille displays
Line range hint
26-34: Major bug fixes
- Fixes for Microsoft Word, Excel, and PowerPoint
- Improvements to web browsing in Firefox and Chrome
- Various fixes for braille display support
Line range hint
36-46: Important changes for developers
- New extension points added
- Changes to speech functions and SSML support
- Updates to the NVDA Controller Client library
- Deprecations and removals of various APIs
Developers should review these changes carefully to ensure compatibility with their add-ons and integrations.
source/speech/speech.py (1)
Line range hint
457-499: Logic Update Correctly Handles Character Normalization IndividuallyThe introduction of
textIsNormalizedanditemIsNormalizedwithin the loop ensures that each character's normalization state is handled independently. This effectively fixes the issue where a normalized character caused all subsequent characters to be incorrectly reported as normalized.
|
Thanks for fixing this @LeonarddeR. I have another case where normalization does not work as expected:
When the whole line is spelled, "normalized" is not reported; but when I navigate with arrows (left/right), it is. This inconsistency is confusing. I'd expect "Normalized" to be also reported when the line is being spelled. Can you fix this in the same PR or should I open a new issue? |
Co-authored-by: Sascha Cowley <16543535+SaschaCowley@users.noreply.github.com>
Link to issue number:
Fixes #17286
Summary of the issue:
When spelling by line, when NVDA normalizes a character, it would incorrectly report all following characters as normalized.
Description of user facing changes
The above issue is fixed.
Description of development approach
There was an isNormalized boolean that would report all characters as normalized when set to True. I fixed this by creating a variable local to the loop that contains the per character normalization state.
Testing strategy:
Tested str from
Known issues with pull request:
None
Code Review Checklist:
@coderabbitai review
Summary by CodeRabbit
New Features
Bug Fixes
Documentation