geanypy: convert to a proxy plugin

[kugel-]

This allows to drop the custom loader and plugin manager and instead
make use of Geany's new proxy plugin feature, where python plugins
are embedded into the standard plugin manager as first class citizen.

Existing plugins continue to run with one exception: The help and configure
methods have been renamed, and in case of configure the samantics have changed
accordingly to Geany's unified configure dialog for all plugins. So existing
scripts that used either show_help() or show_configure() need to to the following:

1. rename show_help() to help()
   2a) rename show_configure to configure()
   2b) change configure to just return the content widget and remove the creation
   of a custom dialog

The plugin script dir is now $geanylibdir/geany, the same as for native plugns.
Geany loooks only there and doesn't make a difference between native and
sub-plugins.

[kugel-]

@codebrainz ping. I want this for G-P 1.26. But not only this, I have keybindings support in the works too, so we gotta hurry a bit.

Both would make geanypy truly useful.

[elextr]

@kugel- does this require any change to any existing Python plugin for it to continue working?

[kugel-]

Am 7. November 2015 02:20:03 MEZ, schrieb elextr notifications@github.com:

@kugel- does this require any change to any existing Python plugin for
it to continue working?

---

Reply to this email directly or view it on GitHub:
#37 (comment)

@elextr all existing plugins continue to work as-is, without any changes. The only thing is that their configure implementation needs to be updated, and until then you cannot configure them (from single configure to the combined one). But they can read their existing configuration and still function as before

[elextr]

The only thing is that their configure implementation needs to be updated, and until then you cannot configure them

@kugel- so unless changed they don't work properly. Its not appropriate to just commit this change then, since it will break all existing Python plugins without warning.

Python plugin authors need time to convert their plugins. Since the new system is a big improvement there is some incentive to do so, but its inappropriate to force them to do so instantly. And the user may not be the author.

There seems to be two possible options:

1. I think that it may be easier to have two Geanypy plugins in G-P, the existing one and the one with this change (GeanypyNG or Geanypy2.0 :). I acknowledge that both cannot be used at once and that is a downside.

2. @codebrainz has suggested that it may actually be easier for you to detect an old Python plugin and adapt to it (compared to making a complete new Geanypy plugin in G-P). And this way old plugins don't just stop working properly without warning, and both old and new ones can co-exist.

[kugel-]

Oh god, the single configure is deprecated since eternity. Yet we shall still implement workarounds for those?

[kugel-]

Am 7. November 2015 12:03:50 MEZ, schrieb elextr notifications@github.com:

The only thing is that their configure implementation needs to be
updated, and until then you cannot configure them

@kugel- so unless changed they don't work properly. Its not
appropriate to just commit this change then, since it will break all
existing Python plugins without warning.

To be honest, it's not your decision how important compatibility is. It's still @codebrainz plugin and he makes the decision. Besides, IMO compatibility is sufficiently given. And since there is no larger distribution there shouldn't be lot of users affected at all (I expect that mostly the developer is the only user).

There seems to be two possible options:

1. I think that it may be easier to have two Geanypy plugins in G-P,
   the existing one and the one with this change (GeanypyNG or Geanypy2.0
   :). I acknowledge that both cannot be used at once and that is a
   downside.

There is nothing that prevents both be active simultaneously. And then you have a hard time to determine which version of geanypy should handle a script (basically impossible).

2. @codebrainz has suggested that it may actually be easier for you to
   detect an old Python plugin and adapt to it (compared to making a
   complete new Geanypy plugin in G-P). And this way old plugins don't
   just stop working properly without warning, and both old and new ones
   can co-exist.

There is no way this can be achieved. The new_hooks system only supports the combined configure. This cannot be made work with python scripts that create and run their own configure dialog.

[elextr]

On 7 November 2015 at 21:41, Thomas Martitz notifications@github.com
wrote:

Am 7. November 2015 12:03:50 MEZ, schrieb elextr <notifications@github.com

:

The only thing is that their configure implementation needs to be
updated, and until then you cannot configure them

@kugel- so unless changed they don't work properly. Its not
appropriate to just commit this change then, since it will break all
existing Python plugins without warning.

To be honest, it's not your decision how important compatibility is. It's
still @codebrainz plugin and he makes the decision. Besides, IMO
compatibility is sufficiently given. And since there is no larger
distribution there shouldn't be lot of users affected at all (I expect that
mostly the developer is the only user).

​@kugel- on open source projects anyone can provide their input, and as the
creator and still technically maintainer of the downstream Geany-Plugins
port it is important that I comment on changes on upstream that I believe
will cause problems for the users.​

There seems to be two possible options:

1. I think that it may be easier to have two Geanypy plugins in G-P,
   the existing one and the one with this change (GeanypyNG or Geanypy2.0
   :). I acknowledge that both cannot be used at once and that is a
   downside.

There is nothing that prevents both be active simultaneously. And then you
have a hard time to determine which version of geanypy should handle a
script (basically impossible).

​I thought you could only have one copy of Python active in a process at
one time, thats why I didn't think they could be both active at once. I
guess if they agree on PATH and other global structures thats fine, but
when one of the plugins closes it will finalize Python and the other will
be likely to crash.

2. @codebrainz has suggested that it may actually be easier for you to
   detect an old Python plugin and adapt to it (compared to making a
   complete new Geanypy plugin in G-P). And this way old plugins don't
   just stop working properly without warning, and both old and new ones
   can co-exist.

There is no way this can be achieved. The new_hooks system only supports
the combined configure. This cannot be made work with python scripts that
create and run their own configure dialog.

​Ok, so the only way is having two geanypy plugins and only one can be run
at a time?​

—
Reply to this email directly or view it on GitHub
#37 (comment).

[codebrainz]

The last param needs to be casted to GDestroyNotify to stifle a warning.

[codebrainz]

I thought you could only have one copy of Python active in a process at one time

They could in theory share the same Python by each calling Py_Initialize(), but there could be issues with GeanyPy symbols clashing and such.

[codebrainz]

It's still @codebrainz plugin and he makes the decision. Besides, IMO compatibility is sufficiently given.

I don't see why we can't do stuff upstream, especially since it no longer provides releases, per se. If people want to wait a while before importing changes into the GP fork, they can wait as long as desired, or implement a duplicate plugin or whatever.

I tend to agree this shouldn't be in 1.26, at the very least we should send an announcement to the mailing lists to let people know in next release their plugin scripts won't continue to work out-of-the-box and they need modifications. That being said, GeanyPy never really promised any API stability/compatibility guarantees, and it shouldn't since it's still a work-in-progress (ex. needs re-working for GTK3 and Python 3), and it wouldn't be ideal to lock ourselves into API guarantees without any user demand/complaints or anything.

[codebrainz]

There is some documentation in docs directory that needs to be updated too. It's the docs that are hosted here:

https://geanypy.readthedocs.org/en/latest/

IIRC it's generated automatically from Git, so just updating the in-tree docs should do.

[codebrainz]

I'm OK if you want to merge this if you don't mind the changes in the PR I made on your fork. As mentioned in a comment, I'd like to combine geany.plugin.Plugin and geany.pluginbase.PluginBase, but that can be done in a separate branch/PR.

Edit: and also updating the docs :)

[elextr]

On 8 November 2015 at 13:01, Matthew Brush notifications@github.com wrote:

It's still @codebrainz https://github.com/codebrainz plugin and he
makes the decision. Besides, IMO compatibility is sufficiently given.

I don't see why we can't do stuff upstream, especially since it no longer
provides releases, per se. If people want to wait a while before importing
changes into the GP fork, they can wait as long as desired, or implement a
duplicate plugin or whatever.

​Of course, but it was my understanding that you had a script that synced
the G-P version. So long as its not run until after 1.26 release thats
fine.​

I tend to agree this shouldn't be in 1.26, at the very least we should
send an announcement to the mailing lists to let people know in next
release their plugin scripts won't continue to work out-of-the-box and they
need modifications.

​I'll look at adding a warning to G-P geanypy README so it will also show
on the plugins website warning about incompatible API changes between 1.26
and 1.27.

That being said, GeanyPy never really promised any API
stability/compatibility guarantees, and it shouldn't since it's still a
work-in-progress (ex. needs re-working for GTK3 and Python 3), and it
wouldn't be ideal to lock ourselves into API guarantees without any user
demand/complaints or anything.

​Sure, but as you say, at least tell people about it :)​

—
Reply to this email directly or view it on GitHub
#37 (comment).

[codebrainz]

my understanding that you had a script that synced the G-P version

You're probably thinking of the Overview plugin:

https://github.com/codebrainz/overview-plugin/blob/master/gp-update.sh

[kugel-]

@codebrainz i pushed a new version that doesn't introduce a new type, thus simplifed (thanks for the hint to store the pointer in a class-instance space).

[kugel-]

@codebrainz I added one commit that adds your desired compatibility. Hope it's fine now.

[codebrainz]

@kugel- cool, I'll have a look when I get some time.

[sagarchalise]

@kugel- @codebrainz Hello people. I couldn't know if this is the place to discuss this. I have a plugin https://github.com/sagarchalise/geanypy-emmet . I tried using keybindings feature but am getting this traceback with segmentation fault.

Program received signal SIGSEGV, Segmentation fault.
plugin_set_key_group (plugin=0x0, section_name=0x7fff6f298d14 "emmet", 
    count=25, callback=callback@entry=0x0) at pluginutils.c:304
304     Plugin *priv = plugin->priv;
(gdb) bt
#0  plugin_set_key_group (plugin=0x0, section_name=0x7fff6f298d14 "emmet", 
    count=25, callback=callback@entry=0x0) at pluginutils.c:304
#1  0x00007ffff79c2734 in plugin_set_key_group_full (plugin=<optimized out>, 
    section_name=<optimized out>, count=<optimized out>, 
    cb=cb@entry=0x7fff8c9a0900 <call_key>, pdata=0x7fff6ec35b40, 
    destroy_notify=0x7fff8c4cd0b0 <Py_DecRef>) at pluginutils.c:334
#2  0x00007fff8c9a0c32 in PluginBase_set_kb_group (self=0x7fff6ec58d40, 
    args=<optimized out>, kwargs=<optimized out>) at geanypy-plugin.c:421

The code used is in https://github.com/sagarchalise/geanypy-emmet/tree/proxy

[kugel-]

@codebrainz will you ever look at this?

[kugel-]

@sagarchalise you must be running an old version. the backtrace mentions geanypy-plugin.c, line 421. That doesn't exist in the latest patch set.

[sagarchalise]

@kugel- I still get

Program received signal SIGSEGV, Segmentation fault.
plugin_set_key_group (plugin=0x0, section_name=0x7fff8681d6e4 "emmet", 
    count=25, callback=callback@entry=0x0) at pluginutils.c:304
304     Plugin *priv = plugin->priv;
(gdb) bt
#0  plugin_set_key_group (plugin=0x0, section_name=0x7fff8681d6e4 "emmet", 
    count=25, callback=callback@entry=0x0) at pluginutils.c:304
#1  0x00007ffff79c0d04 in plugin_set_key_group_full (plugin=<optimized out>, 
    section_name=<optimized out>, count=<optimized out>, 
    cb=cb@entry=0x7fffe6594230 <call_key>, pdata=0x7fff86803af0, 
    destroy_notify=0x7fffe60cefa0 <Py_DecRef>) at pluginutils.c:334
#2  0x00007fffe6594382 in PluginBase_set_kb_group (self=0x7fff86845f38, 
    args=<optimized out>, kwargs=<optimized out>) at geanypy-pluginbase.c:30

I am testing this with a gtk3 build https://github.com/sagarchalise/geanypy/tree/proxy-gtk3. Is it due to that as well ?

[kugel-]

@sagarchalise your fork is out of date w.r.t. to this patch set. Please update.

@codebrainz please look at this. I think I'll push it in a few days because we're going to miss 1.27 otherwise.

[codebrainz]

@kugel- I have not completely reviewed and this is a pretty big PR with a lot of stuff (new plugin API, keybindings support, etc) so I'd really rather review it. I'm still waiting for my PR on your fork to be included and for the documentation to be updated as per comments above. Pardon me if you did already, sometimes Github doesn't email me about some kinds of activity on PRs.

[codebrainz]

Also, I haven't looked why yet, but Github says this won't merge cleanly.

Edit: if it's just from the G_LOG_DOMAIN commit from yesterday, don't worry about it, it will be trivial to fix when merging.

[kugel-]

@codebrainz please review. I've been asking you to do so for some months now.

btw, I've rebased everything to fix the merge conflict

[codebrainz]

@codebrainz please review. I've been asking you to do so for some months now.

I did an initial review in November and was worried about the compatibility break and finally in January you fixed it. I also made a pull request you ignored, and you still haven't indicated whether you've checked to see if the docs need updating. Then you ignored or incorporated my PR commits without attribution and clobbered all the changes I've reviewed thus far by rebasing (which you know I'm strongly opposed to), making me have to start all over. Now you're pleading with me to spend hours of my free time to test and review this change so it can meet some arbitrary deadline (ie. GeanyPy doesn't release, neither in sync with GP nor at all), and it's unclear if it would even be merged downstream in time for that release, and certainly won't have gotten good testing.

sigh

btw, I've rebased everything to fix the merge conflict

FWIW, please don't rebase any PRs on any of my repositories. I know Geany PRs are ok with wiping out the history, but I'm not, it just makes everything harder for everyone just to save a few bytes/noises in the commit log.

[kugel-]

@codebrainz please have another look, I've updated the docs as well now

[codebrainz]

@kugel- what about kugel-/geany-plugins@0b0f4dc, is it needed here, or only for GP?

[kugel-]

@codebrainz indeed, I added the respective commit.

[codebrainz]

@kugel- was this the only thing out-of-sync with downstream? I can't really remember.

[kugel-]

Checked the diff, GP has a few more fixes (regarding config.h, some minor leak, deprecated functions), while geanypy has the logging module. Here's a complete diff http://pastebin.geany.org/8kxjI/ (expires in a month)