Removing scripts from menu items causes issues with shortcuts

[Mikhail22]

Recently I removed a script from Menu items list (in Plugin configuration GUI, remove button).
Restarted N++, as usual but there is a problem - now in the shortcut mapper the shortcuts
are all out of sync. So the item is removed, but the bindings are not updated properly,
the order of internal IDs is changed but the order of bindings is not correct.

So if I remove the first script from menu item list, all the bindings are shifted.
And this is frustrating - now I need to rebind all shortcuts again.

How to reproduce:

• make some scripts and bind keys.
• remove first script from the menu item list in Configuration GUI
• restart
• check the Shortcut mapper, PS plugin section

I am using PS plugin version 1.0.8

Notepad++ v7.5 (32-bit)
Admin mode : OFF
Local Conf mode : ON
OS : Windows 10 (64-bit)
Plugins : mimeTools.dll NppConverter.dll NppExport.dll PythonScript.dll

PS:
Is there any newer version of the Plugin? I have seen comments here about 1.2 version.
But I don't see any info about newer version and how to install.

[Yaron10]

Is there any newer version of the Plugin? I have seen comments here about 1.2 version.
But I don't see any info about newer version and how to install.

https://github.com/bruderstein/PythonScript/releases

[Mikhail22]

@Yaron10 thanks.
So I have tested version 1.2 with latest N++

Notepad++ v7.5.6 (32-bit)
Build time : Mar 19 2018 - 00:26:59
Admin mode : OFF
Local Conf mode : ON
OS : Windows 10 (64-bit)
Plugins : DSpellCheck.dll mimeTools.dll NppConverter.dll NppExport.dll PythonScript.dll

And I still have the same issue.

Created 2 new scripts, assigned Ctrl+F1 and Ctrl+F4 to them.
Then, after removing first script from menu items list same problem:
the item is gone from shortcut mapper list, but the shortcut is wrong.

Contents of shortcuts.xml has not changed after item removal:

   <PluginCommand moduleName="PythonScript.dll" internalID="7" Ctrl="yes" Alt="no" Shift="no" Key="112" />
   <PluginCommand moduleName="PythonScript.dll" internalID="8" Ctrl="yes" Alt="no" Shift="no" Key="115" />

So the problem that the N++ config is not synced. And I think there is no simple workaround,
I could remove the shortcut from config file manually but it requires to know exact IDs of each script
which is not shown anywhere.

[Yaron10]

@Mikhail22,

You're welcome.

I suppose that @chcg and/or @ClaudiaFrank will look into the issue you've reported.

[ghost]

Confirmed.
The issue here is that the shortcut configuration is handled/stored by npp while the menu is
handled/created by PS.
Unsure how to fix this - ideas are welcome.

[Mikhail22]

@ClaudiaFrank My suggestion is to check whether some other plugins have similar option to register / unregister items in N++ and look how the shortcut mapping behaves. If it is same issue than it should be solved in N++ source code I suppose. Though frankly I've never used any plugins other than PS so can't be very helpful.

[chcg]

I guess typically the other plugins I have seen are not modifying their menu and therefore are not facing such an issue. Maybe it is just a matter of first checking if a shortcut for the menu entry exists and than removing the menu entry. I'm not sure if the plugin itself has access to that information of the shortcut mappings.

Maybe this contains some info on it:

• https://notepad-plus-plus.org/community/topic/12576/list-of-all-assigned-keyboard-shortcuts

Checking how https://github.com/dail8859/LuaScript behaves might give some hint how to handle this issue.

[Mikhail22]

Question: what exactly happens when I press "Remove" button in the configuration GUI?
If the answer is that it does only modify the plugin's own configuration file at runtime
(... \plugins\Config\PythonScriptStartup.cnf)
then I think there is simple solution: just create a standalone tool that manages the items and
assigns the shortcuts, this will save the state in the plugin's config file and save shortcuts to "shortcuts.xml" file directly, just generating corresponding XML block.

I have tried to change PythonScriptStartup.cnf manually to add and remove an item - and it works (though I cant give 100% guarantee about possible side effects).
IMO this is optimal solution (and if other solutions are harder to implement) I am voting for a standalone tool. Anyway those manipulation require restarting N++ so it is even better and more robust.

[ghost]

@Mikhail22
By pressing "Remove" PS removes the menuitem from its internal list only,
by pressing "OK" the current config will be saved aka cnf file will be rewritten and menuitems rebuild.

From my current understanding, this are the following possibilities

1. PS opens the npp dialog to let the user clear the shortcut
2. PS opens the npp dialog and tries to clear the shortcut itself.
3. PS informs by a message that there is a shortcut mapped and it should be cleared by the user
4. PS deletes the entry from the shortcuts.xml itself.

I prefer solution 1, simply because npp is aware that there is something to do.
I, ASSUME, deleting the entry from shortcuts.xml will also work but interacting with
a config file from another entity isn't nice and 100% safe I would say.

@chcg
I guess as LuaScript isn't using menu items as well I haven't found anything useful for PS

[chcg]

Maybe there is a 5. option:

• extend n++ plugin interface to support removal of the shortcut from within a plugin

[ghost]

Would be the best, yes, but from the past experience I guess it takes quite some time before such feature is available.

[Mikhail22]

@ClaudiaFrank I am not sure what options 1, 2, 3 would bring, and what npp dialog you mean?
When I was testing items removal - even if there is no active shortcut for current menu item ,
removing of this item will result in broken mappings for all scripts that were placed after the
current item in the list.
So it is all not so easy. And to use n++ itself to do something like forcing the mapping list rebuild upon
restart - I have no idea how this should go.

As a temporary solution I propose the following: make the PS plugin track the mappings
from the beginning and when item add or removal happens - save them in own config file
in same format as those entries from shortcuts.xml. So this side file should contain always correct
mapping list. Then if an item is removed, and the n++ mappings are broken, at least a user can
manually copy-paste this block from the side file into shortcuts.xml. Yes it still not cool but
much better than what happens now.

[Mikhail22]

@ClaudiaFrank As for Option 4. - I suppose it can work only by closed n++,
since otherwise writing to xml has no effect. Therefore I proposed a standalone tool,
or make the PS config GUI start as a detached process and force closing n++ when the
changes are made.

[ghost]

@Mikhail22 by dialog I mean npps shortcut mapper (run->modify shortcut/delete command)
and shortcuts.xml can be edited during npp runtime as long as there is no action which forces npp
to modify this file.

TO ALL
I did some tests:
Option 1 and 2 cannot completely solve this issue as the old entry keeps staying in shortcuts.xml :-(
So there is a need to remove the shortcut assignment from shortcuts.xml in addition.

Edit: Forget the test - I was ignoring the start index npp reports to PS.

[Yaron10]

Hello Claudia,

3. PS informs by a message that there is a shortcut mapped and it should be cleared by the user

I think this is a perfectly legit solution which many apps would use in similar cases.

Thank you for your work. I appreciate it.

[ghost]

Hi Yaron,
so you mean a message like "In order to avoid wrong shortcut assignments make sure that the correspondig entry in shortcuts.xml is deleted and restart notepad++ afterwards" together with opening the proper shortcuts.xml file ?
But the file is a little bit cryptic for normal users, maybe selecting the proper line is needed as well.
@Mikhail22 what do you think?

[Yaron10]

I think there's no need to open shortcuts.xml at all.
The user has assigned the shortcut via Shortcut Mapper; let them remove it manually.

"Please remove the shortcut to this item (Options -> Shortcut Mapper) and restart Notepad++."

What do you think?

[ghost]

This does not delete the entry in shortcuts.xml and thus next restart will assign the shortcuts wrong.

[Yaron10]

I haven't tested the case. I'll do it now.

[Mikhail22]

@ClaudiaFrank

and shortcuts.xml can be edited during npp runtime as long as there is no action which forces npp
to modify this file.

I can edit and save the Xml, yes, but if I do it while opened n++ and then open and close the
Shortcut mapper - this will assign the mappings from the shortcut mapper table to the in-memory
values. And when I close n++ the in-memory values will be written to Xml, so the things I
have edited in the file directly will be LOST.

So there is a need to remove the shortcut assignment from shortcuts.xml in addition.

Yes! the entry with the corresponding ID must be removed from Xml.
The IDs of added scripts always start from 7 and stored in order.
So if I have 3 scripts added and they have kb shortcuts:

script A <PluginCommand...  internalID="7" ... >  
script B <PluginCommand...  internalID="8" ... >
script C <PluginCommand...  internalID="9" ... >

When I remove the first one "7", this line must be removed and IDs must be RENUMBERED.
the Xml should now be:

script B <PluginCommand...  internalID="7" ... >
script C <PluginCommand...  internalID="8" ... >

AND this should be done with closed n++ because the contents of Xml by opened n++ does NOT
resemble the real-time value of mappings - those are saved into Xml only when n++ is closed.
So to my understanding it is most simple and robust solution.

[Yaron10]

so you mean a message like "In order to avoid wrong shortcut assignments make sure that the correspondig entry in shortcuts.xml is deleted and restart notepad++ afterwards" together with opening the proper shortcuts.xml file ?
But the file is a little bit cryptic for normal users, maybe selecting the proper line is needed as well.

If you assign a shortcut and (at the same session) remove the item, you won't have an entry to select.
Just another case to consider. :)

[ghost]

@Mikhail22 thx for confirming,
Ordering isn't enusred

        <PluginCommand moduleName="PythonScript.dll" internalID="0" Ctrl="yes" Alt="yes" Shift="no" Key="78" />
        <PluginCommand moduleName="PythonScript.dll" internalID="5" Ctrl="yes" Alt="yes" Shift="no" Key="80" />
        <PluginCommand moduleName="PythonScript.dll" internalID="1" Ctrl="yes" Alt="no" Shift="yes" Key="67" />
        <PluginCommand moduleName="PythonScript.dll" internalID="7" Ctrl="no" Alt="no" Shift="no" Key="116" />
        <PluginCommand moduleName="PythonScript.dll" internalID="11" Ctrl="yes" Alt="no" Shift="no" Key="72" />
        <PluginCommand moduleName="PythonScript.dll" internalID="12" Ctrl="no" Alt="no" Shift="no" Key="121" />
        <PluginCommand moduleName="PythonScript.dll" internalID="10" Ctrl="yes" Alt="no" Shift="yes" Key="68" />
        <PluginCommand moduleName="PythonScript.dll" internalID="9" Ctrl="yes" Alt="no" Shift="no" Key="82" />
        <PluginCommand moduleName="PythonScript.dll" internalID="8" Ctrl="yes" Alt="no" Shift="no" Key="116" />
        <PluginCommand moduleName="PythonScript.dll" internalID="13" Ctrl="no" Alt="no" Shift="no" Key="123" />
        <PluginCommand moduleName="PythonScript.dll" internalID="3" Ctrl="no" Alt="no" Shift="yes" Key="27" />

I don't see why this "has to be done with closed npp".
Yes, if a user doesn't do it immediately after removing the menu item from configuration,
there is a potenial risk that the shortcuts.xml gets rewritten by npp.
But isn't this up to the user? I mean the same confusion can come up again if the user doesn't execute
a standalone tool.

[Yaron10]

Claudia,

Would closing NPP and running a script rearranging the shortcuts in shortcuts.xml be quite complex?
If not, you can notify the user that NPP should be restarted (Restart and Cancel).

[Mikhail22]

Ordering isn't enusred

Ok I see. Still seems that user script ID alwys start from 7 and added items become IDs in incremental
order. It's only that they are put in Xml when the shortcut is assigned I suppose.

I don't see why this "has to be done with closed npp".
Yes, if a user doesn't do it immediately after removing the menu item from configuration,
there is a potenial risk that the shortcuts.xml gets rewritten by npp.
But isn't this up to the user? I mean the same confusion can come up again if the user doesn't execute
a standalone tool.

Well if you want to leave that up to the user - then its ok :-D
But actually it is quite expectable that user does not close n++ directly after removing an item
and just continue to work and then say opens Shortcut mapper again - and here it breaks.
So the potential for broken mappings is quite high.

My idea about standalone tool implies it would be the only way to remove items and which
also ensure that n++ is not opened.
But I am just throwing an idea - so not necessarily standalone tool, but somehow make it
impossible for the user to not to close n++ directly after changes are made to the list.
But well, if the plugin config dialog is part of n++ then how you can close n++ without
closing the plugin config dialog?
Or maybe just make only one button "save changes and close n++". But I am not sure the
dialog has enough privileges to close the parent app in this case. Also note that currently it
is possible to keep both config dialog and shortcut mapper dialog opened at the same time.
So there a lot of test cases.

[ghost]

@Mikhail22, you're right, as more and more I think about it, the only PS "save" way to do it is to ignore
the remove for the current session (only if a shortcut is assigned of course) but to do it at the end aka when npp shutsdown.
On npp shutdown - start a new (no child) external process which monitors when npp is closed and
then rewrites shortcuts.xml from PythonScriptStartup.cnf entries.
Hopefully noone does something like "on npp shutdown save config files" or having policies like
"config files are only accessible by npp process".

The only true solution would be @chcg option - npp provides a message which can be used to inform
npp that an assigned shortcut has been invalidated as the corresponding menu item has been deleted
and therefore npp deletes the entry in shortcuts.xml and its internal list on runtime.

[sasumner]

Are people just noticing this issue NOW?? I've known about this one for years (but it seemed from 2014 until just very recently there was no one maintaining P.S., ...so I never complained), and I have some Python code (not Pythonscript) to deal with it...that is, to fix it. :-)

[ghost]

Until I saw the issue opened, I forgot about it but you are right it has been an issue almost from the start.
I thought about a python solution as well, unfortunately, as PS has its own python engine, there is no guarantee that there is a local python installation available.

[sasumner]

Right, I wasn't proposing a real fix for it using an independent Python, just sharing what I do to mitigate the pain of this problem. At least deleting Pythonscripts (assigned to shortcuts) isn't an everyday thing, so this pain is infrequent. Doesn't lessen the severity, though! :-)

[ghost]

Asked for a new feature - notepad-plus-plus/notepad-plus-plus#4674.

[chcg]

@ClaudiaFrank Thanks for creating the N++ issue.

[ghost]

PR created - notepad-plus-plus/notepad-plus-plus#4687

[Yaron10]

👍
Thank you Claudia.

[Mikhail22]

@ClaudiaFrank Thanks a lot for keeping your eye on it! That would be really good once it works. Your work is appreciated!

[sasumner]

PR created

Yes, good work...but will anything happen with the PR...

[ghost]

but will anything happen with the PR...

Time will tell

[chcg]

@ClaudiaFrank Did you already impl the pythonscript part based on the N++ PR?

[ghost]

If you are asking whether I did test based on the PR, then yes I did but
it isn't part of the current code or recent commits.

[chcg]

@ClaudiaFrank See preparation at https://github.com/chcg/PythonScript/tree/issue_92, if your N++ PR get merged.

[chcg]

N++ PR merged, needs next N++ release (>7.5.8) to be checked with the impl here.

[sasumner]

Too bad @ClaudiaFrank left Notepad++ for good before seeing this come to fruition! :-(

[chcg]

Fixed with Pythonscript v1.3 and now available https://notepad-plus-plus.org/news/notepad-7.5.9-released.html

[dinkumoil]

@chcg

Since you were able to solve this problem it would be nice if you could help in this discussion.