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

Add support for Sublime Text 3 #4

Closed

Conversation

jpotterm
Copy link

Imports need to be different for version 2 vs version 3 because ST3 changes the python path. ordereddict.py needs to be in a sub-package because all top level modules are loaded as plugins and it uses python 2 specific classes.

…ion 2 vs version 3 because ST3 changes the python path. ordereddict.py needs to be in a sub-package because all top level modules are loaded as plugins and it uses python 2 specific classes.
@jpotterm
Copy link
Author

This time I've tested on both ST2 and ST3 and it should work for each of them.

@thijsdezoete
Copy link
Owner

Aww, now you're changing code inside the isort file, which makes it impossible(read: hard) to update the isort plugin at a later time. Why did you not just change the import in #3 ?

@jpotterm
Copy link
Author

@graingert I don't think there's any advantage to using try catch as opposed to checking the sublime version. Also if we did it that way, we could end up accidentally importing the wrong thing. For example "import isort.isort" might succeed in both ST2 and ST3 but would import different things. In ST2 it might import the isort module and in ST3 it might import the isort package.

@thijsdezoete "code" seems like a strong word for what I changed; I only changed the imports. In order to keep the imports the same as in the original isort, I'd have to mess with sys.path, which I don't want to do because that could break other plugins. For example, if we add our version of pies to the path, then if anyone else has pies on the path, they will conflict. With a little restructuring, I could probably make it so you'd only need to change things like from natsort import natsorted to from .natsort import natsorted. Would that be better?

@thijsdezoete
Copy link
Owner

@jpotterm The idea that I'd like to stick with is that we keep dependencies 'as they are'. This is because it's already really bad that they're just there, without being a git submodule or any other fancy way of getting them there.
So whenever an update is being done on any dependencies, we(me, you, or any other contributor) can just replace the folder of the specific dependency without thinking about how the dependencies have been tied/glued to the project.

@jpotterm
Copy link
Author

@thijsdezoete I know why you'd prefer not to change the dependencies. My point is that if you insist on not changing the dependencies then I cannot think of a way to fix this bug without introducing worst problems. In my opinion you have to choose between changing those dependencies and ST3 support. I someone else can find a solution I overlooked then more power to them.

@thijsdezoete
Copy link
Owner

@jpotterm Someone else upgraded all the dependencies, and made the plugin ST3 compatible. Thanks for your time and effort on this feature. Looking forward to your next pull request! 👍! :)

@jpotterm
Copy link
Author

Good to hear!

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