Skip to content

New command: Move review cursor to position marked as start for copy (#1969)#9925

Merged
feerrenrut merged 3 commits into
nvaccess:masterfrom
accessolutions:i1969-moveToSelectionStart
Oct 8, 2019
Merged

New command: Move review cursor to position marked as start for copy (#1969)#9925
feerrenrut merged 3 commits into
nvaccess:masterfrom
accessolutions:i1969-moveToSelectionStart

Conversation

@JulienCochuyt

@JulienCochuyt JulienCochuyt commented Jul 15, 2019 •
edited by feerrenrut
Loading

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #1969

Summary of the issue:

There is currently no way to tell which review position has been marked as the starting point for a select or copy operation.

Description of how this pull request fixes the issue:

Add a new command to move the review cursor to the position previously marked as starting point.

Testing performed:

Tested from source in a terminal, an editor and a web browser.

  1. On the the licence agreement dialog for NVDA.
  2. Use arrow keys to move the caret to the 'G' from GNU.
  3. Press NVDA+F9 to mark the start.
  4. Press the down arrow key several times to move to a new line.
  5. Press NVDA+shift+F9 to move review cursor to start mark
  6. Press Numpad 6 and Numpad 4 to review words either side of the start mark to get context.

Known issues with pull request:

Change log entry:

Section: New features

A command has been added to move the review cursor to the position previously set as start marker for selection or copy: NVDA+shift+F9

LeonarddeR
LeonarddeR suggested changes Jul 15, 2019
View reviewed changes

@LeonarddeR LeonarddeR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please use the script decorator for this. script_review_currentSymbol contains a good example.

JulienCochuyt added a commit to accessolutions/nvda that referenced this pull request Jul 15, 2019
@JulienCochuyt
Use @script decorator instead of legacy construct (nvaccess#1969)
401e6db 
Re: nvaccess#9925 (review)
@JulienCochuyt

JulienCochuyt commented Jul 15, 2019

Copy link
Copy Markdown
Contributor Author

Please use the script decorator for this. script_review_currentSymbol contains a good example.

Done. My bad, I had not noticed new global commands did follow the new construct.
Shall we some day refactor the whole GlobalCommands class for better readability?
I think the new construct is a plus, but mixed constructs are confusing for newcomers.

@LeonarddeR

LeonarddeR commented Jul 15, 2019

Copy link
Copy Markdown
Collaborator

Done. My bad, I had not noticed new global commands did follow the new construct.

No problem at all.

Shall we some day refactor the whole GlobalCommands class for better readability?

I recall @feerrenrut said somewhere that he'd rather not do this, as it also introduces the risk of breaking things.

@JulienCochuyt JulienCochuyt force-pushed the i1969-moveToSelectionStart branch from 401e6db to e39527c Compare July 15, 2019 14:10
JulienCochuyt added a commit to accessolutions/nvda that referenced this pull request Jul 15, 2019
@JulienCochuyt
Use @script decorator instead of legacy construct (nvaccess#1969)
e39527c 
Re: nvaccess#9925 (review)
@JulienCochuyt

JulienCochuyt commented Jul 15, 2019

Copy link
Copy Markdown
Contributor Author

e39527c: Rebased onto latest master

@JulienCochuyt JulienCochuyt force-pushed the i1969-moveToSelectionStart branch from e39527c to 2281bab Compare August 13, 2019 11:23
@JulienCochuyt

JulienCochuyt commented Aug 13, 2019

Copy link
Copy Markdown
Contributor Author

Rebased onto latest master and linted.
@LeonarddeR, could you please update your review when you have time? It seems rebasing does not allow to reconcile.

@dpy013

dpy013 commented Oct 7, 2019

Copy link
Copy Markdown
Contributor

Is this PR ready to merge? I saw that he is now in an approved state.
thanks

feerrenrut
feerrenrut approved these changes Oct 8, 2019
View reviewed changes

@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.

Thanks @JulienCochuyt

@feerrenrut feerrenrut merged commit c7a00ab into nvaccess:master Oct 8, 2019
@nvaccessAuto nvaccessAuto added this to the 2019.3 milestone Oct 8, 2019
feerrenrut added a commit that referenced this pull request Oct 8, 2019
@feerrenrut
Update changes file for PR #9925
df44fdf
@JulienCochuyt JulienCochuyt deleted the i1969-moveToSelectionStart branch October 9, 2019 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@LeonarddeR LeonarddeR LeonarddeR approved these changes

@feerrenrut feerrenrut feerrenrut approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2019.3

Development

Successfully merging this pull request may close these issues.

jump to selection start marker

5 participants

@JulienCochuyt @LeonarddeR @dpy013 @feerrenrut @nvaccessAuto