Skip to content

Fix for Issue #974. Double clicking on Mocha tests opens external editor.#978

Closed
jcansdale wants to merge 1 commit intomicrosoft:masterfrom
jcansdale:mocha_line0_fix
Closed

Fix for Issue #974. Double clicking on Mocha tests opens external editor.#978
jcansdale wants to merge 1 commit intomicrosoft:masterfrom
jcansdale:mocha_line0_fix

Conversation

@jcansdale
Copy link
Copy Markdown
Contributor

Issue #974

Bug

Double clicking in Mocha test causes JavaScript file to open in external editor.

Fix

Jumping to line: 0, column: 0 seems to cause this. Changed default to line: 1, column: 1.

…nal editor. Changed default to line: 1, column: 1.
@msftclas
Copy link
Copy Markdown

msftclas commented Jun 3, 2016

Hi @jcansdale, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, MSBOT;

@mjbvz
Copy link
Copy Markdown
Contributor

mjbvz commented Jun 6, 2016

Thanks for reporting this issue and finding a fix so quickly. The change looks good overall.

One nitpicky question, should it be line 1, col 0 instead line 1, col 1? I don't think the current solution will cause any problem in most cases, but make sure to test opening a blank file this way to see if that does the correct thing as well.

@jcansdale
Copy link
Copy Markdown
Contributor Author

One nitpicky question, should it be line 1, col 0 instead line 1, col 1?

Good question! It looks like 'NodejsTestInfo.SourceColumn' where it gets stored isn't actually used anywhere. Visual Studio uses 1 based for lines and columns. The v8 debugger and TypeScript package on NPM uses 0 based for both. I guess we can assume it should be consistent.

Maybe code on the JavaScript side should work with 0 based lines/columns and only code on the C#/Visual Studio side should work with 1 based? Otherwise I can see this issue creeping in again and again when new frameworks are added.

@jcansdale
Copy link
Copy Markdown
Contributor Author

This PR is obsolete if you choose to accept PR #982 instead (which would be my preference).

@mjbvz
Copy link
Copy Markdown
Contributor

mjbvz commented Jun 7, 2016

Merged in #982

@mjbvz mjbvz closed this Jun 7, 2016
@jcansdale jcansdale deleted the mocha_line0_fix branch June 7, 2016 00:16
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.

3 participants