Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

command: Fix reload command to correctly initialize and reload all runtime files #3062

Merged
merged 5 commits into from
Mar 22, 2024
Merged

command: Fix reload command to correctly initialize and reload all runtime files #3062

merged 5 commits into from
Mar 22, 2024

Conversation

JoeKar
Copy link
Collaborator

@JoeKar JoeKar commented Nov 30, 2023 •
edited
Loading

This fix will prevent duplicating runtime files in the moment reload and it will now reload the plugins as well.
Now it needs to be discussed if it's really a good idea to export reload to the Lua plugins, because this could lead to unexpected behavior within the plugins.

Fixes #2795
Fixes #3044
Fixes #3059

@dustdfg
Copy link
Contributor

dustdfg commented Dec 1, 2023 •
edited
Loading

Do I correctly understand that if my plugin has variable, after reload it will go to it is default state? If yes, it is only guess and should be checked but it will break filemanager plugin. It opens pane and than handles interactions with it via hooks. They depends on a variable tree_view from the plugin. If it is erased, hooks won't be able to work as expected. And I think that proper filemanager reload means uninit everything, close "managed" pane...

Does micro have hook like uninit or stop or close?

In general I am against of giving plugins ability to call reload on their own.

Now it needs to be discussed if it's really a good idea to export reload to the Lua plugins, because this could lead to unexpected behavior within the plugins.

I just thought, it seems that reload for everything can be unexpected for plugins and especially for user that decided to make reload with opened plugins. Did you meant it? That plugins are built with idea that user will shoot his foot with just reload? You are right. I can propose to add parameters for reload like reload all, reload plugins, reload colors...

@JoeKar
Copy link
Collaborator Author

JoeKar commented Dec 1, 2023 •
edited
Loading

Does micro have hook like uninit or stop or close?

It uses the deinit callback/hook, which I didn't add so far. Good point.

In general I am against of giving plugins ability to call reload on their own.

Maybe we can have both. In the moment the user calls the command reload then this should reload plugins as well, because they are included in the runtime files. This includes the use case in which someone writes a plugin in micro and want it to being available after calling reload. The plugins on the other side currently only receive ReloadConfig as exported function, which might be handy to reload the configuration only, which is expected.

This then wouldn't even violate the documentation:

@JoeKar JoeKar requested a review from dmaluka March 15, 2024 17:31
@dustdfg dustdfg mentioned this pull request Mar 17, 2024
Open
dmaluka
dmaluka reviewed Mar 17, 2024
View reviewed changes
internal/action/command.go Outdated Show resolved Hide resolved
dmaluka
dmaluka reviewed Mar 22, 2024
View reviewed changes
internal/config/rtfiles.go Outdated Show resolved Hide resolved
@JoeKar
rtfiles: Remove the unnecessary init function
d906e39 
With this modification the InitRuntimeFiles() and InitPlugins() (if needed)
must be called first, otherwise uninitialized runtime file variables are most
likely.
@JoeKar JoeKar merged commit a57d29a into zyedidia:master Mar 22, 2024
3 checks passed
@JoeKar JoeKar deleted the fix/reload-cmd branch March 22, 2024 19:47
dmaluka added a commit to dmaluka/micro that referenced this pull request Mar 26, 2024
@dmaluka
Don't initialize plugins and user settings in tests
4a6c1db 
Adding InitRuntimeFiles() and InitPlugins() to buffer_test.go in
PR zyedidia#3062 (instead of just initializing runtime vars with empty values,
as it was before) seems to cause sporadic failures of MacOS build tests
on github, with crashes in various places but all beginning with lots of
plugin failures:

2024/03/26 00:16:23 Plugin does not exist: autoclose at autoclose : &{autoclose autoclose <nil> [runtime/plugins/autoclose/autoclose.lua] false true}
2024/03/26 00:16:23 Plugin does not exist: comment at comment : &{comment comment <nil> [runtime/plugins/comment/comment.lua] false true}
2024/03/26 00:16:23 Plugin does not exist: diff at diff : &{diff diff <nil> [runtime/plugins/diff/diff.lua] false true}
2024/03/26 00:16:23 Plugin does not exist: ftoptions at ftoptions : &{ftoptions ftoptions <nil> [runtime/plugins/ftoptions/ftoptions.lua] false true}
...

I suppose tests should not rely on plugins, and more importantly, should
not be affected by the contents of ~/.config/micro/ on the host.
@dmaluka dmaluka mentioned this pull request Mar 26, 2024
Merged
@JoeKar JoeKar mentioned this pull request Apr 2, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmaluka dmaluka dmaluka approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@JoeKar @dustdfg @dmaluka