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

Implements Global state #1179

Merged
merged 10 commits into from
Dec 20, 2016
Merged

Conversation

vikramthyagarajan
Copy link
Contributor

Holds state of repeat "." actions and search globally (across editors), and added tests for each in their respective test files.

Should fix #1081 and #1156 issues

@johnfn
Copy link
Member

johnfn commented Dec 20, 2016

Wow, very impressive work!

* Caveat: Use only it tests. This promise does not timeout. Awaiting this promise
* in extension code is dangerous
*/
export async function waitForTabChange(): Promise<void> {
Copy link
Member

@johnfn johnfn Dec 20, 2016

Choose a reason for hiding this comment

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

Whoa, thanks for adding this!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you :)

@@ -124,6 +121,41 @@ export class VimState {
public currentFullAction: string[] = [];

/**
* Getters and setters for changing global state
*/
public get searchStatePrevious(): SearchState[]{
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding a bunch of shims for global state, can we just move these directly onto the globals object and allow consumers to use these getters/setters directly?

Copy link
Member

Choose a reason for hiding this comment

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

So instead of saying vimState.searchState they would say vimState.globalState.searchState?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think that makes sense. Will do

@johnfn
Copy link
Member

johnfn commented Dec 20, 2016

Minor request and we should be good to go here!

// import * as vscode from 'vscode';
import { SearchState } from './searchState';
import { RecordedState } from '../mode/modeHandler';
// import { Position } from './../motion/position';
Copy link
Member

Choose a reason for hiding this comment

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

can remove unused imports here

@@ -30,6 +30,23 @@ export async function waitForCursorUpdatesToHappen(): Promise<void> {
});
}

/**
* FOR TESTING PURPOSES ONLY
Copy link
Member

Choose a reason for hiding this comment

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

if you want you can add a guard to use if Globals.isTesting is set to true, not a big deal

@johnfn johnfn merged commit ac87146 into VSCodeVim:master Dec 20, 2016
@johnfn
Copy link
Member

johnfn commented Dec 20, 2016

Thanks again for the awesome work.

@xconverge
Copy link
Member

And tests!

@vikramthyagarajan vikramthyagarajan deleted the global-state branch December 21, 2016 07:22
This pull request was closed.
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.

Search is unique to file
3 participants