Call plugin_loaded on target modules

[Varriount]

From my investigations into nim-lang/NimLime#121 , I've found that AutomaticPackageReloader doesn't appear to call the plugin_loaded function on target modules. This function needs to be called, as several plugins perform necessary startup actions there.

[randy3k]

This line should execuate the plugin_loaded function, afaik.

[Varriount]

The reload_plugin function only calls plugin_loaded on the target module - not on any of the module's dependencies.
The normal sublime text plugin loading logic loops through sys.modules and calls the plugin_loaded function (if the module has such a function).

[randy3k]

Maybe you are right. But it is too not straightforward to determine which modules should be reloaded, In addition, I don’t think we should reload any dependencies.

[FichteFoll]

(External) dependencies are not part of the package and should not be reloaded. When you modify those, you should probably del sys.modules['dep_name'] (and probably all submodules as well) in the console.

You could make a case for AutomaticPackageReloader also being able to reload a dependency iff you are working on one.

[Varriount]

@randy3k The modules that need plugin_loaded called are the same ones that are being reloaded. The work of finding and reloading them is already being done.

I've been able to patch AutomaticPackageReloader on my system to call plugin_loaded by using this patch:

175c176,179
<                 return module.__loader__.load_module(name)
---
>                 result = module.__loader__.load_module(name)
>                 if "plugin_loaded" in result.__dict__:
>                     result.plugin_loaded()
>                 return result

Although this code isn't quite right either - the modules loaded via reload_plugin have their plugin_loaded function being called twice.

[randy3k]

I would argue that NimLime should not modify the python search path for this modules. You should put a file in the root directory and import all the submodules to the root. In this way, it avoids having multiple plugin_loaded definitions and calls.

See for example how it is done in Terminus: https://github.com/randy3k/Terminus/blob/master/main.py

[Varriount]

@randy3k I've since modified NimLime to not modify the search path (look at the restructure branch).

[randy3k]

I guess you should also refactor this part of the codebase https://github.com/Varriount/NimLime/blob/restructure-for-st3/core/commands/__init__.py#L31 and import the submodules relatively. Why do you need this file at the first place?

[Varriount]

@randy3k That code loops through all submodules and exports the Sublime Text plugin classes. That way, when a new command or event listener is added to the commands directory, it is automatically picked up by Sublime Text via the star import in NimLime.py.

The import flow process looks something like this:

NimLime.py -- Star Import --> core/commands/__init__.py -- Dynamic Import --> core/commands/*.py

Is this causing some sort of problem with AutomaticPackageReloader? The dynamic import process uses the same import functions that Sublime's sublime_plugin.py plugin loader uses.

[Thom1729]

Is this issue live, or does #20 resolve it?