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

AddLibrary plugin to easily download libs #6203

Merged
merged 2 commits into from Apr 4, 2017
Merged

AddLibrary plugin to easily download libs #6203

merged 2 commits into from Apr 4, 2017

Conversation

caiofsouza
Copy link
Contributor

Please provide the following information:

Also make sure you:

  1. Used "tags": true and not "branch": "master" (versioning docs)
  2. Ran the tests (tests docs)

@FichteFoll
Copy link
Collaborator

FichteFoll commented Apr 3, 2017

On ST3, use json.loads(sublime.load_resource("Packages/AddLibrary/Libraries.json")) instead of directly reading the file, since that will fail when the package is installed in archived state, which is the default. This method doesn't exist in ST2, so you either have to add a fallback or remove ST2-support.

You don't import Request in the ST2 import code.

from .Libraries import Libraries as a relative import will only function in ST3. For ST2, you need to use from Libraries import Libraries for root-level modules (yes, this is not ideal).

My suggestion is to remove ST2 support, since only about 95% 5% of ST users are using it.

@math2001
Copy link
Contributor

math2001 commented Apr 3, 2017

My suggestion is to remove ST2 support, since only about 95% of ST users are using it.

You probably meant the opposite no? 95% are using st3

Is there any reason to use json.loads instead sublime.decode_value? The second one accepts trailing commas...

@FichteFoll
Copy link
Collaborator

@math2001 thanks for spotting that.

sublime.decode_value can parse trailing commas and C-style comments, so it is better feature-wise. It is however not available in ST2 and I never benchmarked it on how it compares to json.loads.

@caiofsouza
Copy link
Contributor Author

Hello @FichteFoll ,
Thanks for the suggestions. This is my first plugin and I'm learning how to build that and the methods of sublime API. And, of course, the sublime.load_resource seems me better. About support to ST2, I'll change the support to only ST3 :D

@caiofsouza
Copy link
Contributor Author

@math2001 I did not use sublime.decode_value because I did not known this method, nothing special...

Change support to only ST3
@FichteFoll FichteFoll merged commit d91b443 into wbond:master Apr 4, 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.

None yet

3 participants