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

fix #510 #659

Merged
merged 3 commits into from
Aug 28, 2016
Merged

fix #510 #659

merged 3 commits into from
Aug 28, 2016

Conversation

xconverge
Copy link
Member

We update everything better now when text editors are changed

@xconverge xconverge changed the title fix #510 [WIP] fix #510 Aug 27, 2016
@xconverge
Copy link
Member Author

So this works pretty well...however....it bombs on the tests.

I tracked it to the fact that now that I am calling getAndUpdateModeHandler(), it will create modehandlers that have withTesting set to FALSE for the tests.

Is there a way to disable this event handler for just the tests? :/

Any advice would be great...

@johnfn
Copy link
Member

johnfn commented Aug 27, 2016

Can you move our isTesting flag to a global somewhere? That's something I should have done back when I made modehandler per-editor rather than global itself.

@xconverge
Copy link
Member Author

where would be the best place to put a global like that in extension.ts? And then where is the top most level of testing where I could set it, testSimplifier.ts?

@johnfn
Copy link
Member

johnfn commented Aug 27, 2016

You may actually want to start a new file called globals.ts and follow the same pattern i did in taskqueue.ts to export it.

testSimplifier is OK, but not all of our tests use that infrastructure. Maybe there's a better place somewhere? Is it possible to latch onto when mocha starts running tests somehow? Some sort of global "before all tests" thing? I'm not sure!

@xconverge xconverge changed the title [WIP] fix #510 fix #510 Aug 28, 2016
@xconverge
Copy link
Member Author

xconverge commented Aug 28, 2016

got it, let me know how it looks, works for me :)

@@ -12,6 +12,7 @@ import { showCmdLine } from './src/cmd_line/main';
import { ModeHandler } from './src/mode/modeHandler';
import { TaskQueue } from './src/taskQueue';
import { Position } from './src/motion/position';
import './src/globals';
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 importing everything from globals into the namespace, can you do something like

import { globals } from './src/globals'

and then check globals.isTesting?

@johnfn
Copy link
Member

johnfn commented Aug 28, 2016

More small comments.

@xconverge
Copy link
Member Author

Let me know if that hits any of the marks you were referring to...

@johnfn
Copy link
Member

johnfn commented Aug 28, 2016

This looks great. Can you just fix up those merge conflicts and we should be good to go here.

@xconverge
Copy link
Member Author

woops bad merge..

@johnfn johnfn merged commit 7a3ef28 into VSCodeVim:master Aug 28, 2016
@johnfn
Copy link
Member

johnfn commented Aug 28, 2016

Another great change. 0.1.8 is shaping up to be the most bug free release in VSCodeVim history! (sort of.)

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.

2 participants