search

Gubaer / josm-scripting-plugin

Task automation in the OpenStreetMap editor JOSM
https://gubaer.github.io/josm-scripting-plugin
GNU General Public License v3.0
26 stars 9 forks source link

Add new "add to toolbar" checkbox #80

Closed bagage closed 5 years ago

bagage commented 5 years ago

Maybe partly fixes #24?

Adding action to toolbar has some benefits, including the fact that you can then assign a shortcut to it via toolbar preferences.

Gubaer commented 5 years ago

Thanks, @bagage! I'm looking forward to merge this patch - sorry, I didn't respond earlier.

I run some tests with JOSM 15195 (then latest). Sometimes (but not always) the toolbar button added for a script file doesn't survive a restart. It just isn't included in the tooblar anymore after the restart. In the JOSM log file I see the following message 

2019-06-26 18:45:18.066 INFO: Could not load tool definition scripting/run(scriptingFilename=/<redacted>/hello_world_script.py){name=Run script from file '/<redacted>/hello_world_script.py'}

Do you have any idea what could be the reason for this?

bagage commented 5 years ago

No worries @Gubaer, it's not that urgent :).

Regarding the log, this is as expected since JOSM tries to resolve toolbar buttons' action at startup (before loading the plugins): it does not know scripting plugin actions yet. But once plugins are loaded, the toolbar buttons are resolved again (maybe only the missing ones), and there JOSM detects that this is scripting plugin and it is happy with it.

I'm not sure we can get rid of this log - but maybe this toolbar wasn't intended for plugin at first so…? maybe we should ask @don-vip about that?

For the restart issue, the button is removed from the toolbar if the associated action cannot be resolved even after plugins load. So either you are using an older version of josm scripting and thus the action is not exposed and JOSM get rid of it... otherwise there is a issue (race condition?) that I didn't saw. Do you have a reproducible scenario?

Thanks!

Gubaer commented 5 years ago

Thanks @bagage! I today merged your pull request and published plugin build 30795