randy3k / AutomaticPackageReloader

Automatically reload submodules while developing a Sublime Text package.
MIT License
38 stars 13 forks source link

WIP: Rewrite using sublime_plugin.load_module. #32

Closed Thom1729 closed 5 years ago

Thom1729 commented 5 years ago

Sublime 3189 provides a new sublime_plugin.load_module() function. This allows us to load a module without using Sublime's reloading code, which may let us substantially simplify the reloading code. This PR is a first draft of what such a simplification may look like.

The current reloading algorithm goes like this:

  1. Resolve dependencies, find modules to reload.
  2. Unload plugins (using sublime_plugin.unload_module).
  3. Remove modules from sys.modules.
  4. Reload plugins (using sublime_plugin.reload_plugin). This should reload the modules.

Step 2 is not desirable by itself, but it's necessary because sublime_plugin.reload_plugin both reloads the given module and registers its contents with Sublime, and the reloading part works differently depending on whether the given module is in sys.modules. If it is, then reload_plugin will call unload_module, which we already called in step 1. Therefore, it has proved necessary to manually remove items from sys.modules to invoke the second case in reload_plugin. That case, in turn, calls importlib.import_module directly rather than using __import__, which means that we have to temporarily add a new finder to sys.meta_path to intercept the top-level plugin imports and actually reload modules.

This PR uses the new sublime_plugin.load_module, which only registers the given module's contents with Sublime and does not attempt to unload the plugin contents or reload the module. (In essence, load_module is the counterpart to unload_module, whereas reload_plugin is more like unload_plugin.) This allows the following algorithm:

  1. Resolve dependencies, find modules to reload.
  2. Unload plugins (using sublime_plugin.unload_module).
  3. Reload modules.
  4. Load plugins (using sublime_plugin.load_module).

Because load_module won't trigger module reloads, we can do it ourselves. The implementation in the PR does this by patching __import__ in a similar way to importing_fromlist_aggresively (and subsuming the latter). This ensures that modules are reloaded in the correct order without needing to modify either sys.modules or sys.meta_path.

The advantages of this approach are:

Disadvantages may include:

This PR isn't ready; I need to do more testing and write the fallback code. The approach should be pretty stable, though, so I'd appreciate any comments or feedback.

randy3k commented 5 years ago

Awesome job. You mentioned that the current reloader code does not work in some edge cases, would you provide some insights?

Fallback may not be necessary, we could put a requirement of build 3189 on Package Control and show an error message to users who clone the repo directly but with build <3189.

FichteFoll commented 5 years ago

You could also just ship an older build for older versions through package control's release channels (like for st2).

randy3k commented 5 years ago

You could also just ship an older build for older versions through package control's release channels (like for st2).

I think it is what I 've said. Perhaps my english has broken.... nevermind

FichteFoll commented 5 years ago

No, you just didn't mention to keep the old version and ship it for older at builds.

randy3k commented 5 years ago

oh, you are right. I was thinking it is the same process....

randy3k commented 5 years ago

Let's target it for ST4! 😄

randy3k commented 5 years ago

@Thom1729 If you don't have anything to add, I am going to merge this.

PS: I have already pined the last tag for pre-3189 builds.