zyedidia / micro

A modern and intuitive terminal-based text editor
https://micro-editor.github.io
MIT License
24.4k stars 1.16k forks source link

Backwards incompatible changes in plugin API #3209

Closed Andriamanitra closed 3 months ago

Andriamanitra commented 3 months ago

https://github.com/zyedidia/micro/commit/4ffc2206eeea4a7d58bdf283e35cdf28b29b0b42 removed some bindings that were previously available for plugins to use. This means that all plugins (such as my LSP plugin https://github.com/Andriamanitra/mlsp/issues/1) that relied on them are broken on master.

I would suggest reverting the change as I don't see any harm in keeping these bindings around. If they are to be removed I think there should at least be a deprecation period to avoid disruptions for users. Especially so since the alternative was very recently introduced – as a plugin author I can't even update the plugin to use the alternative because it doesn't yet exist in the latest release (2.0.13), not to mention the distro packages which tend to lag behind by a few versions (for example Ubuntu LTS is still on 2.0.9).

JoeKar commented 3 months ago

At least from my perspective (which doesn't need to be common) fully reverting it isn't what we want in the first place, since the intention was and is to provide a more abstracted API were we don't need to care about some specific micro internals take place in parallel. But I give you fully right, that breaking APIs with no changing period isn't the best practice.

So my suggestion would be the reintroduction of these interfaces (partly revert), BUT with our own wrapper function where we can directly expose a deprecation warning. With this we should be save for the adaptation phase with the master as well as the current release.

Any objections?

dmaluka commented 3 months ago

I've uploaded PR #3211 reverting https://github.com/zyedidia/micro/commit/4ffc2206eeea4a7d58bdf283e35cdf28b29b0b42. With #3211 your plugin should work as fine as it used to. I mean, as unsafe as it used to. From what I can see, your timer callback doesn't synchronize with the rest of your plugin, or with micro, whatsoever. What if e.g. activeConnections or openFiles changes in the meantime, while your time callback is running? It doesn't look like you update activeConnections in an atomic way, and so on. So I'd suggest to switch to the new (safe) API as soon as possible.

dmaluka commented 3 months ago

I suppose you could also check at runtime if the new interface is available:

if type(micro.After) == "function" then
    -- use new API
else
    -- use old API
end