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

Implement "Go Last" #33715

Merged
merged 1 commit into from
Sep 6, 2017
Merged

Implement "Go Last" #33715

merged 1 commit into from
Sep 6, 2017

Conversation

brandonbloom
Copy link
Contributor

This implements a "Go Last" command. This command will navigate either forward or backwards in history to the last entry in the history stack you were looking at. I've implemented this in service of implementing '' and `` for vim mode: VSCodeVim/Vim#1089

@mention-bot
Copy link

@brandonbloom, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bpasero and @egamma to be potential reviewers.

@msftclas
Copy link

msftclas commented Sep 2, 2017

@brandonbloom,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

@msftclas
Copy link

msftclas commented Sep 2, 2017

@brandonbloom, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

@bpasero
Copy link
Member

bpasero commented Sep 4, 2017

@brandonbloom trying to understand the purpose of the command:

This command will navigate either forward or backwards in history to the last entry in the history stack you were looking at.

Is the idea that you just always remember the index of the last history operation by the user (either going forward or backward) so that you can jump back to that index? It is not clear to me how it would ever jump forward because once you go back and then navigate to another editor, the future history is lost.

@bpasero bpasero added this to the On Deck milestone Sep 4, 2017
@brandonbloom
Copy link
Contributor Author

My goal is to implement a pair of vim commands that I use regularly: VSCodeVim/Vim#1993

I typically use the line-wise '', but it can be defined in terms of the character-wise `` operation. The VIM docs has this to say about ' and `:

Jumping to a mark can be done in two ways:
1. With ` (backtick):	  The cursor is positioned at the specified location
			  and the motion is |exclusive|.
2. With ' (single quote): The cursor is positioned on the first non-blank
			  character in the line of the specified location and
			  the motion is linewise.

And this to say about the '' and `` chords:

							*''* *``*
''  ``			To the position before the latest jump, or where the
			last "m'" or "m`" command was given.  Not set when the
			|:keepjumps| command modifier was used.
			Also see |restore-position|.

In these terms, both any cursor motion, including navigating forwards or backwards, counts as a "jump".

You jump somewhere, and then tap '' as many times as you like to flip back and forth between the place you came from and the place you went to. If you only execute the command once, it's no different than <C-o>, which is the "Navigate Back" command. If you execute it multiple times, it is essentially the same as alternating between "Navigate Back" and "Navigate Forward". This is extremely useful for quickly comparing two places within a single file.

VSCodeVim does not currently maintain its own "jump list", it relies on Code's navigation history. I implemented the simplest subset of the functionality I could think of, in the lowest risk way I could think of. That is: I just maintain the last place in history that you were. If there's any chance that last place is invalidated (such as when truncating history), you probably navigated somewhere, so "Navigate Back" is a reasonable thing to do.

Does this explanation clear things up?

@bpasero
Copy link
Member

bpasero commented Sep 6, 2017

You jump somewhere, and then tap '' as many times as you like to flip back and forth between the place you came from and the place you went to.

Isn't the same possible by using the Ctrl+Tab keys and releasing to jump between MRU editors?

panel-red

@brandonbloom
Copy link
Contributor Author

brandonbloom commented Sep 6, 2017

  1. No. That's not the same thing for a few reasons. Most importantly, that won't work on one file in one editor. This is common when fiddling with imports while working, or when referring to some types at the top of a file, for example.

  2. The goal is to provide even a crude approximation of a pair of very commonly used vim bindings.

@@ -368,6 +368,7 @@ registry.registerWorkbenchAction(new SyncActionDescriptor(FocusPreviousGroup, Fo
registry.registerWorkbenchAction(new SyncActionDescriptor(FocusNextGroup, FocusNextGroup.ID, FocusNextGroup.LABEL, { primary: KeyChord(KeyMod.CtrlCmd | KeyCode.KEY_K, KeyMod.CtrlCmd | KeyCode.RightArrow) }), 'View: Focus Next Group', category);
registry.registerWorkbenchAction(new SyncActionDescriptor(NavigateForwardAction, NavigateForwardAction.ID, NavigateForwardAction.LABEL, { primary: null, win: { primary: KeyMod.Alt | KeyCode.RightArrow }, mac: { primary: KeyMod.WinCtrl | KeyMod.Shift | KeyCode.US_MINUS }, linux: { primary: KeyMod.CtrlCmd | KeyMod.Shift | KeyCode.US_MINUS } }), 'Go Forward');
registry.registerWorkbenchAction(new SyncActionDescriptor(NavigateBackwardsAction, NavigateBackwardsAction.ID, NavigateBackwardsAction.LABEL, { primary: null, win: { primary: KeyMod.Alt | KeyCode.LeftArrow }, mac: { primary: KeyMod.WinCtrl | KeyCode.US_MINUS }, linux: { primary: KeyMod.CtrlCmd | KeyMod.Alt | KeyCode.US_MINUS } }), 'Go Back');
registry.registerWorkbenchAction(new SyncActionDescriptor(NavigateLastAction, NavigateLastAction.ID, NavigateLastAction.LABEL, { primary: null, win: null, mac: null, linux: null }), 'Go Last');
Copy link
Member

Choose a reason for hiding this comment

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

Can be written as:

registry.registerWorkbenchAction(new SyncActionDescriptor(NavigateLastAction, NavigateLastAction.ID, NavigateLastAction.LABEL), 'Go Last');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@bpasero
Copy link
Member

bpasero commented Sep 6, 2017

@brandonbloom can you also create an issue in VSCode repo so that we can resolve it with this PR. The issue best explains this feature so that it can be tested during end game (e.g. what is the expected behaviour).

@brandonbloom
Copy link
Contributor Author

Done. See #33902

@bpasero bpasero merged commit 41cd56a into microsoft:master Sep 6, 2017
@bpasero
Copy link
Member

bpasero commented Sep 6, 2017

Thanks 👍

@bpasero bpasero modified the milestones: September 2017, On Deck Sep 6, 2017
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants