search

xabikos / vscode-javascript

Contains the code snippets for JavaScript (ES6) development in VS Code editor
MIT License
332 stars 137 forks source link

Expose configuration key for semicolon occurrence #35

Open felicio opened 7 years ago

felicio commented 7 years ago

Very much work-in-progress, but I'm hoping this will steer down a conversation about future form of this PR, whose attempt is to provide the user with an option of opting out of resolving snippets only with semicolons.

I guess the main question is: Is this request welcomed?

Resolves #27

felicio commented 7 years ago

@xabikos, I'm very curious what do you think about the direction this PR is taking. Please, let me know if I can be of any help on the maintainer side.

xabikos commented 7 years ago

@felicio My apologies for the late reply.

Thanks for the contribution and the PR.

I haven't been involved on developing a real extension for VS Code but if I understand correctly the PR changes entirely the nature of the package and from simple snippets transforms it to proper extension for the editor.

This requires to have an extra setting to specify if the snippets should include a semicolon or not, is this correct. We could provide a default value for that and then give the user the option to change it, right?

I like the idea to be honest, as I see more and more people don't use semicolons any more.

felicio commented 7 years ago

That's correct. You did the harder part, honestly.

It does that by resolving the snippets on-the-fly while responding to a change in configuration property called snippets-javascript.semi, by default set to true. Obviously, I like the idea too, however there are two things about this particular solution that I'd like to point out.

Note: To see it in action one can fetch this branch, open in it in VS Code and Debug > Start Debugging.

xabikos commented 7 years ago

@felicio thanks for the explanation and the additional check in that improves the extension.

I have a couple of questions though.

Regarding the setting is it going to be a regular setting that is accessible the regular VS Code settings page (Ctrl + ,). If not we should do it in this way if possible.

Now about the VS Code engine I don't consider this a problem as you mentioned that the current version of the editor is 1.18.0. To ask as minimum version 1.8.0 is more than reasonable. In any case once we publish this the major change is going to be increased to indicate that.

I will try to spend some time and look into the code. Please let me know if you plan to push any additional changes, so to know if I can merge this. I guess that I can also create the extension locally and install in from there to test it.

felicio commented 7 years ago

is it going to be a regular setting that is accessible the regular VS Code settings page

Yes, it is. Personally I use "snippets-javascript.semi": false.

let me know if you plan to push any additional changes

The solution is final. Let me just squash some commits, which I'll do right away.

felicio commented 7 years ago

Updated. I'll gladly answer any further questions here.