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

Some small improvements #45

Merged
merged 2 commits into from
Mar 21, 2017
Merged

Some small improvements #45

merged 2 commits into from
Mar 21, 2017

Conversation

RSDuck
Copy link
Contributor

@RSDuck RSDuck commented Mar 18, 2017

["[", "]"],
["(", ")"]
],
"autoClosingPairs": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

I dont sure that this is good idea to use json configuration together with code based in nimMain.ts, also autoClosingPairs is marked as Deprecated in https://github.com/Microsoft/vscode/blob/master/src/vs/vscode.d.ts#L2786
May be it will more reasonable use declaration in code for better type safety?

Copy link
Contributor Author

@RSDuck RSDuck Mar 20, 2017

Choose a reason for hiding this comment

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

Is it really deprecated if it's inside a json configuration? Because the source code states:

/**
* **Deprecated** Do not use.
*
* @deprecated * Use the the autoClosingPairs property in the language configuration file instead.
*/

That's the reason why I created a json language config file anyway. And here in the official manual they still recommend autoClosingPairs in json configs. Also e.g. the Python extension(integrated into VSCode) depends on autoClosingPairs, otherwise they would have the same problem.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok

@kosz78 kosz78 merged commit 460bf1d into pragmagic:master Mar 21, 2017
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