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

Add file extension to link & clean up code #1

Merged
merged 5 commits into from Oct 14, 2018

Conversation

Projects
None yet
2 participants
@spikespaz
Contributor

spikespaz commented Oct 14, 2018

I noticed some things that apparently your linter didn't catch. The quotes were a mix of single and double, your parentheses and indentation were strange, and you were using function() {} and () => {}. Your function call path was messy and ready to break at the slightest change.

I fixed these issues to make the code more consistent as well as removing the sample comments that came from wherever you copied the original file from.

Interestingly enough, I couldn't require("vscode"), even though I can in every other extension, ever. So that part is not like the others because I'm lazy and I gave up.

Additionally, I made sure that Hastebin uses the correct syntax highlighting by adding the file extension of the open document's filename to the end of the assembled Hastebin URL. This fixed some issues that I had with D code.

You were missing the "license" field in your package.json, so I fixed that too.

Great extension, thanks!

@HamzaAnis

This comment has been minimized.

Owner

HamzaAnis commented Oct 14, 2018

Thank you very much for the PR and cleaning up the code. I was using the default linter however good catch for this. I'll test it soon and will merge it.

@@ -1,68 +1,71 @@
'use strict';
// The module 'vscode' contains the VS Code extensibility API

This comment has been minimized.

@HamzaAnis

HamzaAnis Oct 14, 2018

Owner

I generated the starting code using Yo code so these were here from start.

This comment has been minimized.

@spikespaz

spikespaz Oct 14, 2018

Contributor

I see. Well, "use strict"; is still there and the comments didn't add much, I hope you are fine with them being gone.

This comment has been minimized.

@HamzaAnis

HamzaAnis Oct 14, 2018

Owner

Yes I am absolutely fine. You did a good job improving it and fixing the mistakes.

} else if (!vscode.window.activeTextEditor.selection.isEmpty) {
code = vscode.window.activeTextEditor.document.getText(vscode.window.activeTextEditor.selection);
} else {
} else if (vscode.window.activeTextEditor.selection.isEmpty) {

This comment has been minimized.

@HamzaAnis

HamzaAnis Oct 14, 2018

Owner

Easier to read now. 👍

}
uploadText(content).then((response: any) => {

This comment has been minimized.

@HamzaAnis

HamzaAnis Oct 14, 2018

Owner

This is more convenient. ✔

@HamzaAnis HamzaAnis merged commit 9444fdc into HamzaAnis:master Oct 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment