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.

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

Make Plugins Admin to be aware of minimum and maximum Notepad++ version a plugin can deal with #5741

Closed
dinkumoil opened this issue Jun 4, 2019 · 11 comments
Assignees
Labels
accepted plugin pluginAdmin Issues related to the new plugin admin system introduced with N++ 7.6

Comments

@dinkumoil
Copy link
Contributor

dinkumoil commented Jun 4, 2019

Plugins Admin should take into account the minimum and maximum Notepad++ version a plugin can deal with

Occasionally the API of Notepad++ respectively Scintilla changes. The last time this happened in Npp v7.7 when Scintilla has been updated to v4.1.4. That introduced an updated version of the SCNotification struct and leads for Notepad++ 64 bit to some struct members now having a length of 64 bit. As a consequence all 64 bit plugins using the SCNotification struct have to be updated because they supposedly will cause crashes of Notepad++. See >>> this topic <<< at the community forum.

Currently the plugin list is part of a Notepad++ installation and as such it is version specific. But this could change in the future as it is on the agenda to make the plugin list independent from a certain Notepad++ version. Instead Plugins Admin should be able to work with an online plugin list.

At that time it will cause problems when a user installs a plugin via Plugins Admin that is not suitable for his current version of Notepad++.

Expected Behavior

The plugin list should be extended by a field containing the minimum Notepad++ version a plugin requires. Plugins Admin should process this field to be able to prevent the installation and even loading of plugins not suitable for the current version of Notepad++.

Additionally a field for the maximum Notepad++ version a plugin can deal with could be useful.

If one or both of these fields are empty or non-existent, Plugins Admin should skip the according version check.

Actual Behavior

Plugins Admin loads and installs all plugins a user requests it to. It is not able to prevent the installation/loading of plugins unsuitable for the current Notepad++ version.

Applies to

Notepad++ v7.7

@dinkumoil dinkumoil changed the title Make Plugins Admin to be aware of minimum and maximum Notepad++ version a plugin needs Make Plugins Admin to be aware of minimum and maximum Notepad++ version a plugin can deal with Jun 4, 2019
@pnedev
Copy link
Contributor

pnedev commented Jun 4, 2019

Just to add a suggestion:

Another approach might be to introduce a Notepad++ plugins API version.
Then each plugin in the nppPluginList list needs to state which API version it is built for and PluginsAdmin to match that to the current Notepad++ version API.

This way we'll avoid specifying both minimum and maximum Notepad++ version as the maximum cannot be known at the time the plugin is released (we don't know when in the future the API will change).

@dinkumoil
Copy link
Contributor Author

@pnedev

This would also require a maximum plugins API version.

It could be that some detail of the plugin API will be removed or its implementation will change, that's why I thought about a maximum version number. Of course, this can not be known before, but when it happens a plugin author could change his entry in the plugin list and define a maximum Notepad++ version for his plugin. This way he gains some time to implement the required fix/change in his plugin without burden users with a freshly/already installed plugin that causes crashes. This is the situation we have right now.

I will change the feature request to also prevent Plugins Admin to load already installed plugins which do not meet the min/max version requirements.

@pnedev
Copy link
Contributor

pnedev commented Jun 4, 2019

@dinkumoil ,

You are right.

@MetaChuh MetaChuh added plugin pluginAdmin Issues related to the new plugin admin system introduced with N++ 7.6 labels Jun 4, 2019
@MetaChuh
Copy link

MetaChuh commented Jun 4, 2019

@dinkumoil

i believe this is just a temporary state of transition valid for 7.7 as well as 7.7.1 and better to be re-evaluated at a later time.

as the plugin list is currently deployed together with a notepad++ release only, older versions like 7.6.6 or lower, will only see the plugins that were available at that time, rendering an additional scintilla version depending api filter unnecessary.

if all goes as expected, everything will settle down, once sci 414 has been around for enough time, to allow active plugin developers to adapt their plugins.

note: i can completely understand your motivations, but we all should keep a long term perspective, to keep a mutually satisfying balance, e.g. for evaluations between requests to upgrade scintilla, and predictable requests as a result of upgrading scintilla.

many thanks and best regards.

@dinkumoil
Copy link
Contributor Author

dinkumoil commented Jun 4, 2019

@MetaChuh

I've made the whole feature request under a long term perspective. As long as there is no online plugin list a min/max version number is nearly useless. But it should be introduced before an online list is available.

@chcg
Copy link
Contributor

chcg commented Dec 26, 2020

Also with the current system of pluginAdmin to bond the update of the list to the N++ release there is no guarentee that the plugin is functional with the N++ version. The plugins are checked via a CI job at https://github.com/notepad-plus-plus/nppPluginList that they are still available for download, but there is no check of the API conformance against the current N++ version once the plugin got integrated to the plugin list.

So maybe an additional simple N++ startup check together with the downloaded plugin could be added to the CI job. As the probably needs quite some time it might be not run on each update of the list. Maybe once with each N++ update.

@donho
Copy link
Member

donho commented Dec 26, 2020

It's a good point and sorry for not being aware of this issue before.
It's rather the responsibility of Notepad++ but not of PluginsAdmin to control plugins' compatibility (to me).
Notepad++ plugin API tries always to be retro-compatible so no obsolete API so far.
However, the change of SCNotification is problematic - I have no idea how to check this info from Notepad++.
If you have any suggestion, please let me know.

@ArkadiuszMichalski
Copy link
Contributor

@chcg

So maybe an additional simple N++ startup check together with the downloaded plugin could be added to the CI job. As the probably needs quite some time it might be not run on each update of the list. Maybe once with each N++ update.

If we have already downloaded certain .zip versions (cache), checking only the launch NPP takes as in this results:
totalTime: 1 [min] 34 [s] << 32-bit
totalTime: 1 [min] 14 [s] << 64-bit

I also try closing the dialog boxes created by plugins at startup, so these times can probably be twisted (if it is not necessary).

@chcg
Copy link
Contributor

chcg commented Mar 19, 2021

@ArkadiuszMichalski Could we go on with that at notepad-plus-plus/nppPluginList#250. Do you have a script available which could be used for the appveyor CI job?

@ArkadiuszMichalski
Copy link
Contributor

@chcg Ok, let's move the discussions over there

@donho
Copy link
Member

donho commented Mar 5, 2022

Done in a06b404

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted plugin pluginAdmin Issues related to the new plugin admin system introduced with N++ 7.6
Projects
None yet
Development

No branches or pull requests

6 participants