Skip to content

Python Console: Jump to previous/next result (#9784)#9785

Merged
feerrenrut merged 19 commits into
nvaccess:masterfrom
accessolutions:i9784
May 7, 2021
Merged

Python Console: Jump to previous/next result (#9784)#9785
feerrenrut merged 19 commits into
nvaccess:masterfrom
accessolutions:i9784

Conversation

@JulienCochuyt

@JulienCochuyt JulienCochuyt commented Jun 21, 2019

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #9784

Summary of the issue:

In the output pane of the Python Console, it can be tedious to inspect a series of lengthy output results.

Description of how this pull request fixes the issue:

Provide key bindings to jump to the previous/next result, select a whole result and clear the output pane.

Testing performed:

Ported from an add-on in use for a few months [EDIT] years.

Known issues with pull request:

Change log entry:

Python Console: When the focus is on the output pane, alt+up/down jumps to the previous/next output result (add shift for selecting). control+l clears the output pane.

@JulienCochuyt

Copy link
Copy Markdown
Contributor Author

Added the control+L binding to clear the output and rebased onto latest master.

@JulienCochuyt

Copy link
Copy Markdown
Contributor Author

990d28f: Rebased onto latest master

@JulienCochuyt

Copy link
Copy Markdown
Contributor Author

Rebased onto latest master and linted.

@JulienCochuyt

Copy link
Copy Markdown
Contributor Author

Working on another subject, I think I found a way to improve this PR: Take advantage of the fact the Python Console is a singleton to control it from the NVDA AppModule.
It would allow to use alt+up/down instead of control+up/down and even allow the gesture to be configured (or aliased to a braille gesture).
It might also allow to add alt+shift+up/down for result selection, but I have first to test if the corresponding NVDAObject correctly detects the selection anchor.
What do you think?

@LeonarddeR

Copy link
Copy Markdown
Collaborator

@JulienCochuyt

What do you think?

I think this makes sense.

@JulienCochuyt

Copy link
Copy Markdown
Contributor Author

@LeonarddeR: Just pushed the refactoring using the NVDA AppModule.
Default gestures are now alt+(shift+)up/downArrow and still control+l to clear the output pane.
Handling of the selection anchor works as expected when extending the current selection.

@feerrenrut feerrenrut added audience/nvda-dev PR or issue is relevant to NVDA / Add-on developers component/NVDA-GUI feature ux labels Apr 8, 2020
@JulienCochuyt

Copy link
Copy Markdown
Contributor Author

@dingpengyu wrote:

Is there any new progress in pr?

From my side, everything is done. I'm using this feature on a daily basis through custom patching for about two years.
The PR has been doubly approved, further improved since then, and kept in sync with the master branch...
Please advise if I can do anything more.

@LeonarddeR

Copy link
Copy Markdown
Collaborator

well, if @michaelDCurran approved it, I don't think there's anything holding this back for a merge. May be it was missed somehow.

@feerrenrut

Copy link
Copy Markdown
Contributor

It looks there have been a number of changes since Mick last reviewed. I'll request another review so he can look over the new changes.

@lukaszgo1

Copy link
Copy Markdown
Contributor

@JulienCochuyt Would you be able to resolve conflicts here?

Comment thread source/appModules/nvda.py Outdated
@CyrilleB79

Copy link
Copy Markdown
Contributor

While at it, please also update the PR's initial descriptions:

  • new key binding description
  • clear() / ctrl+L in change log

@JulienCochuyt JulienCochuyt requested a review from a team as a code owner April 23, 2021 02:32
@cary-rowen

Copy link
Copy Markdown
Contributor

This is great.

@JulienCochuyt

Copy link
Copy Markdown
Contributor Author

@CyrilleB79 wrote:

While at it, please also update the PR's initial descriptions:

  • new key binding description
  • clear() / ctrl+L in change log

Done. Sorry for the lengthy delay.

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've merged in master, and resolved conflicts. I'd like to understand this isPrevFocusOnNvdaPopup, but I'll need more ecplanation.

Comment thread source/gui/__init__.py
@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 8d767bd238

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@XLTechie

XLTechie commented May 6, 2021 via email

Copy link
Copy Markdown
Collaborator

@feerrenrut feerrenrut merged commit c22509b into nvaccess:master May 7, 2021
@nvaccessAuto nvaccessAuto added this to the 2021.1 milestone May 7, 2021
@JulienCochuyt JulienCochuyt deleted the i9784 branch May 7, 2021 06:54
@JulienCochuyt

Copy link
Copy Markdown
Contributor Author

@feerrenrut,
As a follow-up, I realize the new script category "Python Console" assigned to the new scripts on the NVDA app module should probably also be assigned to the global command activatePythonConsole (currently "Tools").
My question is "Where should I put the constant definition?"
Currently, it is an attribute of appModules.nvda as it is only used there.
My best bet would be to store it instead as an attribute of the pythonConsole module and reference it from both the app module and the global commands, turning their current late imports of the module into module-level imports.
Alternatively, it could be stored as an attribute of the globalCommands module - as most are - and imported into the app module from there.
What do you prefer?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

audience/nvda-dev PR or issue is relevant to NVDA / Add-on developers component/NVDA-GUI feature ux

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python Console: Jump to previous/next result

10 participants