-
-
Notifications
You must be signed in to change notification settings - Fork 747
UI Automation in Windows Console: limit blank lines in review and initial word movement support #9647
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
UI Automation in Windows Console: limit blank lines in review and initial word movement support #9647
Changes from 11 commits
41d4816
53ecca9
a9575ba
1cf0563
6083ded
768fe0f
9a8835c
bc55777
6f52524
c2386b9
c016ab1
6557f04
4e9bbe0
b12cbc9
e4e96ef
d00a15b
1df93fa
9fef531
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,6 +4,8 @@ | |||||||||||
| # See the file COPYING for more details. | ||||||||||||
| # Copyright (C) 2019 Bill Dengler | ||||||||||||
|
|
||||||||||||
| import ctypes | ||||||||||||
| import NVDAHelper | ||||||||||||
| import speech | ||||||||||||
| import time | ||||||||||||
| import textInfos | ||||||||||||
|
|
@@ -17,10 +19,12 @@ | |||||||||||
|
|
||||||||||||
| class consoleUIATextInfo(UIATextInfo): | ||||||||||||
| _expandCollapseBeforeReview = False | ||||||||||||
| _isCaret = False | ||||||||||||
|
|
||||||||||||
| def __init__(self, obj, position, _rangeObj=None): | ||||||||||||
| super(consoleUIATextInfo, self).__init__(obj, position, _rangeObj) | ||||||||||||
| if position == textInfos.POSITION_CARET: | ||||||||||||
| self._isCaret = True | ||||||||||||
| if isAtLeastWin10(1903): | ||||||||||||
| # The UIA implementation in 1903 causes the caret to be | ||||||||||||
| # off-by-one, so move it one position to the right | ||||||||||||
|
|
@@ -31,6 +35,121 @@ def __init__(self, obj, position, _rangeObj=None): | |||||||||||
| 1 | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| def move(self, unit, direction, endPoint=None): | ||||||||||||
| oldRange=None | ||||||||||||
| if not self._isCaret: | ||||||||||||
|
||||||||||||
| if not self._isCaret: | |
| if self.basePosition != textInfos.POSITION_CARET: |
Outdated
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 don't know how caching works here, but it might make sense to do something like this:
| if visiRanges.length > 0: | |
| visibleRangesLength = visiRanges.Length | |
| if visibleRangesLength > 0: |
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.
What exactly is the advantage of doing this? We only use the value once per function call.
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.
You're also getting the length for lastVisiRange
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.
Expand should use similar code to move. E.g.:
If unit is word, then use _getCurrentOffset and _getWordOffsets and manually move the start and end of the range to have it correctly bound the current word.
Then, there is no need to change what text is exposed as the textRange will be correct.
The advantage of this is there is less to track, it is easier to read, and most importantly, later comparisons or movements of this textInfo won't behave like it is expanded to line even though the text returned is word.
There may be a question around performance: but I'd like to see the code changed to my proposed way before we then consider if optimisations are needed at all.
LeonarddeR marked this conversation as resolved.
Show resolved
Hide resolved
Outdated
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.
In most if not all cases, you're calling _getCurrentOffsetInThisLine and then _getWordOffsetsInThisLine, which both copy self and expand to a line. May be lineInfo could be an argument to both functions to avoid this? Of course, it should still only be used when self is collapsed within that line
Outdated
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.
Recent experiences with Notepad++ made me a bit worried about endless while loops. Could you limit this somehow?
Would something like this work?
| while True: | |
| while charInfo.compareEndPoints( | |
| lineInfo, | |
| "startToEnd" | |
| ) <= 0: |
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 don't think this is necessary, see below.