Threading breaks usefulness of sublime_plugin.EventListener in ST3 about sassbeautify HOT 19 CLOSED

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.

from sassbeautify.

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!

from sassbeautify.

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

import time
time.sleep(5)

from sassbeautify.

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?

from sassbeautify.

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.

from sassbeautify.

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

from sassbeautify.

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).

from sassbeautify.

Awesome! Thanks!

from sassbeautify.

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 :)

from sassbeautify.

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?

from sassbeautify.

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.

from sassbeautify.

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.

from sassbeautify.

ST3 on Windows 7. Maybe this is a Windows oddity...
On Feb 10, 2014 12:59 PM, "Richard Willis" [email protected] 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
.

from sassbeautify.

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

from sassbeautify.

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.

from sassbeautify.

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.

from sassbeautify.

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.

from sassbeautify.

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

from sassbeautify.

Thanks for testing!

from sassbeautify.