Skip to content

Prefer Difflib in scrolling LiveText objects#12974

Merged
seanbudd merged 7 commits into
nvaccess:masterfrom
codeofdusk:prefer-difflib-old-conhost
Nov 4, 2021
Merged

Prefer Difflib in scrolling LiveText objects#12974
seanbudd merged 7 commits into
nvaccess:masterfrom
codeofdusk:prefer-difflib-old-conhost

Conversation

@codeofdusk

@codeofdusk codeofdusk commented Oct 21, 2021

Copy link
Copy Markdown
Contributor

Link to issue number:

Twitter thread.

Summary of the issue:

In LiveText objects that are constrained to onscreen text (i.e. where text scrolls off the screen), DMP is sometimes unable to correctly calculate new text, leading to choppy and inconsistent new text reporting.

Description of how this pull request fixes the issue:

Prefer Difflib in pre-FORMATTED UIA console (including legacy mode) and DisplayModelLiveText objects. Users can override this choice in advanced settings for testing or if DMP really provides a supperior experience in their situation.

Testing strategy:

Tested the following scenarios:

Application User setting Algorithm used
FORMATTED Windows Console Automatic DMP
FORMATTED Windows Console Diff Match Patch DMP
FORMATTED Windows Console Difflib Difflib
IMPROVED Windows Console Automatic Difflib
IMPROVED Windows Console Diff Match Patch DMP
IMPROVED Windows Console Difflib Difflib
END_INCLUSIVE Windows Console Automatic Difflib
END_INCLUSIVE Windows Console Diff Match Patch DMP
END_INCLUSIVE Windows Console Difflib Difflib
Legacy Windows Console Automatic Difflib
Legacy Windows Console Diff Match Patch DMP
Legacy Windows Console Difflib Difflib
Windows Terminal Automatic DMP
Windows Terminal Diff Match Patch DMP
Windows Terminal Difflib Difflib
PuTTY Automatic Difflib
PuTTY Diff Match Patch DMP
PuTTY Difflib Difflib

Known issues with pull request:

None known.

Change log entries:

== Bug Fixes ==

  • Improved consistency of output reading in terminal programs.
    • Note that in some situations, when inserting or deleting characters in the middle of a line, the characters after the caret may again be read out.

== Changes for Developers ==

  • diffHandler.get_dmp_algo and diffHandler.get_difflib_algo renamed diffHandler.prefer_dmp and diffHandler.prefer_difflib respectively.

Code Review Checklist:

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

@codeofdusk codeofdusk force-pushed the prefer-difflib-old-conhost branch 3 times, most recently from 21b7ee5 to fde3656 Compare October 21, 2021 20:26
@codeofdusk codeofdusk force-pushed the prefer-difflib-old-conhost branch from fde3656 to 37cdec6 Compare October 21, 2021 20:29
@codeofdusk codeofdusk marked this pull request as ready for review October 21, 2021 20:53
@codeofdusk codeofdusk requested review from a team as code owners October 21, 2021 20:53
Comment thread source/NVDAObjects/behaviors.py Outdated
Comment thread source/NVDAObjects/behaviors.py Outdated
Comment thread source/NVDAObjects/window/winConsole.py
Comment thread source/NVDAObjects/window/__init__.py
@seanbudd seanbudd self-assigned this Oct 27, 2021
codeofdusk and others added 2 commits October 27, 2021 00:20
Co-authored-by: Sean Budd <seanbudd123@gmail.com>

@seanbudd seanbudd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Qchristensen - what are your thoughts on the docs changes?

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 2bb94a9c51

@AppVeyorBot

This comment has been minimized.

@AppVeyorBot

This comment has been minimized.

@Qchristensen Qchristensen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Makes sense, good work.

@codeofdusk codeofdusk requested a review from seanbudd November 4, 2021 02:12
@seanbudd seanbudd merged commit fd9ae8c into nvaccess:master Nov 4, 2021
@nvaccessAuto nvaccessAuto added this to the 2022.1 milestone Nov 4, 2021
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.

5 participants