Skip to content
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

Update fast_diff_match_patch to 2.0.1 #15514

Merged
merged 2 commits into from
Sep 26, 2023

Conversation

codeofdusk
Copy link
Contributor

Link to issue number:

None

Summary of the issue:

The diff-match-patch-python package changed its name to fast_diff_match_patch.

Description of how this pull request fixes the issue:

Update to the new package name, changing nvda_dmp accordingly.

Testing strategy:

Verified that diffing in consoles works.

Known issues with pull request:

None known

Change log entry:

None needed

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@codeofdusk codeofdusk requested a review from a team as a code owner September 25, 2023 07:43
@codeofdusk codeofdusk requested a review from seanbudd September 25, 2023 07:43
@AppVeyorBot
Copy link

See test results for failed build of commit dd19a463ee

@seanbudd seanbudd added this to the 2024.1 milestone Sep 26, 2023
@seanbudd
Copy link
Member

I think we should always note dependency upgrades either in changes or changes for developers. Can you please add a changelog entry for "changes for developers" in this case?

@codeofdusk codeofdusk force-pushed the fast-diff-match-patch branch from ce6f6d3 to 3970451 Compare September 26, 2023 02:56
Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

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

Thanks @codeofdusk

@seanbudd seanbudd merged commit 69a16cc into nvaccess:master Sep 26, 2023
seanbudd pushed a commit that referenced this pull request Oct 12, 2023
Fix-up of #15513 #15514 .

Description of how this pull request fixes the issue:
Add @codeofdusk attributions for Python 3.11 upgrade
michaelDCurran added a commit that referenced this pull request Feb 12, 2024
seanbudd pushed a commit that referenced this pull request Feb 13, 2024
Fixes #16027
Reverts #15514
This reverts commit 69a16cc.

Summary of the issue:
PR #15514 upgraded diff_match_patch from 1.0.2 to fast_diff_match_patch 2.0.1.
However, the newer version cannot handle particular unicode characters such as 🍰. The diff_match_patch process dies, NvDA can no longer report text changes, and NvDA hangs on exit.

Description of user facing changes
Printing unicode characters such as 🍰 in a terminal withn using diff_match_patch for speaking changes no longer causes NvDA to no longer report text changes and lock up on exit.

Description of development approach
Downgrade back to diff_match_patch 1.0.2.
Adriani90 pushed a commit to Adriani90/nvda that referenced this pull request Mar 13, 2024
Fixes nvaccess#16027
Reverts nvaccess#15514
This reverts commit 69a16cc.

Summary of the issue:
PR nvaccess#15514 upgraded diff_match_patch from 1.0.2 to fast_diff_match_patch 2.0.1.
However, the newer version cannot handle particular unicode characters such as 🍰. The diff_match_patch process dies, NvDA can no longer report text changes, and NvDA hangs on exit.

Description of user facing changes
Printing unicode characters such as 🍰 in a terminal withn using diff_match_patch for speaking changes no longer causes NvDA to no longer report text changes and lock up on exit.

Description of development approach
Downgrade back to diff_match_patch 1.0.2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants