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 to latest nvda_dmp #16508

Merged
merged 5 commits into from May 10, 2024
Merged

Conversation

codeofdusk
Copy link
Contributor

@codeofdusk codeofdusk commented May 8, 2024

Link to issue number:

None

Summary of the issue:

nvda_dmp has received updates for better handling of large diffs, unicode characters, etc.

Description of how this pull request fixes the issue:

Updates nvda_dmp (and its backing diff-match-patch implementation) to the latest version.

Testing strategy:

Daily testing by two active terminal users.

Known issues with pull request:

None known

Change log entry:

== Changes ==

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 marked this pull request as ready for review May 9, 2024 22:15
@codeofdusk codeofdusk requested a review from a team as a code owner May 9, 2024 22:15
@seanbudd
Copy link
Member

seanbudd commented May 9, 2024

please add a change log entry to 2024.3 in changes.md under changes for developers

user_docs/en/changes.md Outdated Show resolved Hide resolved
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 for this @codeofdusk

@seanbudd seanbudd merged commit 4f66fd6 into nvaccess:master May 10, 2024
1 check passed
@nvaccessAuto nvaccessAuto added this to the 2024.3 milestone May 10, 2024
XLTechie pushed a commit to XLTechie/xlnvda that referenced this pull request May 10, 2024
nvda_dmp has received updates for better handling of large diffs, unicode characters, etc.

Description of how this pull request fixes the issue:
Updates nvda_dmp (and its backing diff-match-patch implementation) to the latest versi
@@ -13,6 +13,7 @@

* Component updates:
* Updated Unicode CLDR to version 45.0. (#16507, @OzancanKaratas)
* Updated fast_diff_match_patch (used to detect changes in terminals and other dynamic content) to version 2.1.0. (#16508, @codeofdusk)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure, but this does not seem to be an update but rather standard diff match patch being replaced by fast diff match patch.
@codeofdusk, is it correct? If yes, the change log item should be modified.

@zstanecic
Copy link
Contributor

zstanecic commented May 14, 2024 via email

@Danstiv
Copy link
Contributor

Danstiv commented May 14, 2024

@zstanecic
It's strange that you noticed improvements, because The fix was a little different. And it shouldn't have affect braille.
Perhaps performance has increased due to the transition back to the fast diff match patch, but just in case, check if you have UIA notifications as diffing algorithm selected.

@zstanecic
Copy link
Contributor

zstanecic commented May 14, 2024 via email

@codeofdusk
Copy link
Contributor Author

The change log entry is fine as-is, because:

  • fast-diff-match-patch is a renaming of the existing package (same maintainer, same repo).
  • No changes to Braille code were made. Any performance gains here are incidental.

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.

None yet

7 participants