Better porting to ST3 #67

Merged
merged 1 commit into from Oct 17, 2013

4 participants

@Verhaeg

Saw a pull request from hayd, and tried to improve that

now it uses the Thread module and created a GuardMessageCommand to be able to put the output in the panel. The only thing missing for that is to be able to erase/clear the panel in case of text found.

@hayd

should be a space here for python2 support :) i.e. print (string)

strange since I didn't have problems with that in ST2, is it an older version, perhaps before python 2.7?
AFAIK print became a function starting in 2.7 and if not mistaken, this is the version used by ST2

In any case, thanks for pointing, I'll probably add the same if statement here then ;)

You're quite correct, how embarrassing! That I should really have known.

Don't worry ;) I'm not a Python expert as well, is live and learn! (Sorry if it seemed I was rude, that was definitely not my intention)
And I enjoy these reviews chats ;) often they become a good source of ideas!

@edubkendo

@tbueno wondered what your plans are for either merging this in or doing something else to provide st3 support?
Now that Package Control 2.0 is out, it's actually pretty easy to have an st2 and an st3 branch and you just tell PC which is which and then it does the right thing.

@tbueno
Collaborator

@edubkendo I'll test the solution and if it works, it will be merged soon. Even if we decide to rewrite some parts later to offer better integration with ST3, we should make it compatible now.

@cyphactor cyphactor commented on the diff Oct 2, 2013
@@ -1,6 +1,11 @@
import sublime
import sublime_plugin
-import thread
+try:
@cyphactor
Owner

This seems like really a Python version issue not a sublime text 2 vs. 3 issue.

Would it not be better to use something like the following to get/check the Python version?

>>> import sys
>>> sys.version_info
(2, 5, 2, 'final', 0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@cyphactor cyphactor commented on the diff Oct 2, 2013
@@ -19,7 +24,10 @@ def __init__(self):
def set_listener(self, listener):
self.listener = listener
- self.output_view = self.listener.window.get_output_panel('guard')
+ if st_version == 2:
@cyphactor
Owner

Also for this I believe there is the following:

sublime.version()

Looks like it is referenced in both the SublimeText 2 and 3 Plugin API docs.

http://www.sublimetext.com/docs/2/api_reference.html

http://www.sublimetext.com/docs/3/api_reference.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@cyphactor cyphactor commented on the diff Oct 2, 2013
@@ -72,9 +80,15 @@ def start_guard(self):
self.running = True
self.show_guard_view_and_enable_autoshow()
if self.proc.stdout:
- thread.start_new_thread(self.read_stdout, ())
+ if st_version == 2:
@cyphactor
Owner

Again, this seems more like a Python version issue rather than a SublimeText version issue and therefore I think it should use

sys.version_info()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@cyphactor cyphactor commented on the diff Oct 2, 2013
if self.proc.stderr:
- thread.start_new_thread(self.read_stderr, ())
+ if st_version == 2:
@cyphactor
Owner

Again, this seems more like a Python version issue rather than a SublimeText version issue and therefore I think it should use

sys.version_info()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@cyphactor cyphactor commented on the diff Oct 2, 2013
@@ -104,20 +118,24 @@ def append_data(self, data):
clean_data = self.remove_terminal_color_codes(clean_data)
# actually append the data
- self.output_view.set_read_only(False)
- edit = self.output_view.begin_edit()
+ if st_version == 2:
@cyphactor
Owner

This is SublimeText version issue so it should probably use

sublime.version()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@tbueno tbueno merged commit e81c78e into cyphactor:master Oct 17, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment