Threading breaks usefulness of sublime_plugin.EventListener in ST3

[owenleonard]

I've been trying to add an option to this plugin to run SassBeautify on the save of a sass/scss file. I tried adding a sublime_plugin.EventListener class to SassBeautify to call the "sass_beautify" command in the on_pre_save() method (https://www.sublimetext.com/docs/3/api_reference.html#sublime_plugin.EventListener). I had to do a little fiddling to stop an infinite loop of the save calling beautify calling save calling beautify calling...

The next issue I ran into was that the on_pre_save() method actually completes before the "sass_beautify" command runs. In fact, on_post_save() completes before the "sass_beautify" command can run. I'm pretty sure it is because "sass_beautify" runs things on a background thread thus allowing on_pre_save() to complete. I put a print statement in on_pre_save(), on_post_save(), and in ExecSassCommand.run() right inside the try statement. The print statement in ExecSassCommand.run() was the last one to print in the console.

What this means is that I can have it beautify when I save the file, but the changes are technically made after ST3 completes the save, thus the file is left in a modified state.

Given that ST3 runs all plugins in a separate process, is there any plan to create an ST3 version of SassBeautify without the threading?

[badsyntax]

Hi there, thanks for creating this issue. I can certainly see the usefulness of this feature. I made the plugin compile the sass in a different thread because sass is slow and in some cases on slow machines the compilation may take quite a bit of time and thus makes ST unresponsive, which is not good! I understand the issues this causes when trying to run the command as part of a synchronous process. The last thing the beautify command does is to save the file, so it should not be left in a modified state. To be honest though I'm unfamiliar with the events so there's obviously something I'm not understanding. I am going to try code this up and report back when if I have any solutions.

[owenleonard]

I wonder if it still makes ST3 unresponsive given the plugin runs in a separate process, or if that is just ST2. I'm not sure, just thinking aloud (or at least typing out my thoughts). Thanks for the quick response!

[badsyntax]

Yea it definitely does in ST3, I have just tested this like so:

import time
time.sleep(5)

[badsyntax]

I've created a new branch for development of this feature.

This commit shows how it's possible to run the command on_post_save, although it's not very elegant. It would be awesome if there was a way to prevent the on_post_save event when saving, rather than having to resort to using boolean flags to prevent the infinite save >> on_post_save >> save loop.

Otherwise, as far as I can tell, this does what you want it to do. Thoughts?

[owenleonard]

Yep, I understand what you mean about not wanting to resort to using a boolean flag, but I'm not sure how to get around it. Your cleanup commit works great, though.

[badsyntax]

Thanks for testing it. I will probably merge this into master at some point today.

[badsyntax]

I've merged this into master. It will take a little while before you can upgrade the plugin via package control (i'd give it 24hrs).

[owenleonard]

Awesome! Thanks!

[owenleonard]

It appears that you committed 79699e8 which doesn't work, instead of the cleanup work you did in 4315c91. Can you get the cleanup work merged in? I'm not sure how to reopen this issue :)

[badsyntax]

I merged the 2 commits together with a git rebase. Commit 79699e8 contains all the changes I made on this branch.

Can you give some more information as to how it's not working?

[owenleonard]

I set beautifyOnSave to true, but it isn't beautifying the file when I save it. I did notice that in the event listener, you are loading the settings during init like so:

def init(self):
self.settings = sublime.load_settings('SassBeautify.sublime-settings');

The problem is this restricts reading the settings file to ST startup. Any change to the settings file requires either restarting ST or perhaps disabling/enabling the plugin.

If you call sublime.load_settings() inside of on_post_save(), it will get the current setting.

I tried restarting ST and it still isn't beautifying the file when I save it. I can beautify the file if I run the SassBeautify command from the pallette.

If I remember correctly from testing your changes over the weekend, putting sublime.load_settings() in init makes the self.settings.get() call return nothing for beautifyOnSave, thus the sass_beautify command never gets run.

[badsyntax]

This was working on my setup earlier today (osx) and is working fine for me now on Ubuntu. What platform are you testing on? What version of Sublime Text are you using? I will put the settings back into the on_post_save method tomorrow.

[owenleonard]

ST3 on Windows 7. Maybe this is a Windows oddity...
On Feb 10, 2014 12:59 PM, "Richard Willis" notifications@github.com wrote:

This was working on my setup earlier today (osx) and is working fine for
me now on Ubuntu. What platform are you testing on? What version of Sublime
Text are you using? I will put the settings back into the on_post_save
method tomorrow.

Reply to this email directly or view it on GitHubhttps://github.com//issues/33#issuecomment-34675256
.

[badsyntax]

Possibly! Thanks for testing this, I'll make that change tomorrow and try test on Windows.

[owenleonard]

I was thinking about this some more today because I don't like the inefficiency of querying the settings on every save. I thought it would be nice to have an event that tells you when a setting changes. Lo and behold, there is one supported on both ST2 and ST3 - http://www.sublimetext.com/docs/3/api_reference.html#sublime.Settings. I haven't coded it up, but if you'd like me to help I can fork the repo and give it a shot. I don't have access to OSX, but I can test on Win7 and Linux.

[badsyntax]

Seems a bit overkill imo. I don't think it's inefficient to query the settings on_post_save. When loading the settings, "subsequent calls to load_settings with the name base_name will return the same object, and not load the settings from disk again." See load_settings here: http://www.sublimetext.com/docs/3/api_reference.html

And when a user updates their settings file, ST will re-load the settings from disk.

So it seems fine to both load and query the settings on_post_save. I'll try make that change today.

[badsyntax]

I have made that change on master and have released the bugfix. The update will take a little while to come through via package control.

I would really appreciated if you could test this change as it's a real effort for me to test on Windows.

[owenleonard]

I got the update through Package Control and it is working correctly on Win7 + ST3! Thanks for fixing this!

[badsyntax]

Thanks for testing!