[Geany-devel] [PATCH] X session management support

[Eugene Arshinov]

Hi Enrico.

On Tue, 8 Dec 2009 21:06:10 +0100%
Enrico Tröger <enrico.troeger at uvena.de> wrote:

> 
> >  [4.implementation.patch]
> >
> >    The implementation. I did not to extract it to separate source
> >    code files so far, but I'll definitely do it if you wish.
> 
> It might be better but can also be done easily later.
> 

OK. I'll move the code to separate "sm.h" and "sm.c" files and send
here the patch, probably at the weekend.

> 
> >    XSMP requires a Geany instance to have the same XSMP client-ID
> >    when it is restarted by the session manager. I created new
> >    `--libsm-client-id' command-line option in order to specify it
> >    in restart command.
> >
> >    Actually, I looked into claws-mail source code and did not find
> > any code to maintain client-ID there. Maybe maintaining client-ID is
> >    not very important, so I can remove `--libsm-client-id' option if
> >    anyone votes against it.
> 
> On a side note, did you copy any code from Claws? This could be
> problematic as Claws is released as GPLv3 while Geany is GPLv2.
> 

No, I didn't copy any code from claws.

> 
> >  1. Geany session management
> >
> >    I have to use `--no-session' command-line option in restart
> >    command. Please see code comments inside
> > [4.implementation.patch], the "FIXME" section. There I described,
> > why I have to use the option and why it is bad.
> >
> >    There is an easy fix: Geany instance should not save Geany
> > session if the instance is run with `--new-instance' option. I
> > consider this behaviour acceptable.
> 
> Me too.
> 

OK. Then I'll take care of this issue and send here two patches: one
that fixes '--new-instance' option behaviour and one that changes
'--no-session' option handling.

> 
> > [4.implementation.patch].
> >
> >Untested functionality:
> >
> >  1. Building on Windows
> >
> >    I had no opportunity to test building on Windows. Autotools and
> > waf should simply fail to find libsm, thus XSMP support should be
> >    disabled.
> 
> Don't worry. I would just test it but unfortunately I destroyed again
> my Windows test VM by accident...the second time already. Having image
> files laying around on my hard disk seems to be a high risk here...
> More seriously, I don't know how many users are actually compiling
> Geany on Windows themselves but I doubt there are many at all. Once I
> find the time to set up a test Windows box again, I'll give it try.
> 

Thanks in advance.

> 
> >  1. Handle all command-line options
> >
> >    Most of command-line options, specified when running Geany,
> > should go to restart command. Things get little complicated as some
> >    options need special handling (for example, I think that `--line'
> >    and `--column' options should not go to restart command).
> >
> > [...]
> >
> >    I think, the best solution of this code duplication problem is
> > some kind of a "reverse" parser of GOptionEntry's. It does not make
> >    sense to write one when you have 10 options or so, most of which
> >    have string values. But if such a "reverse" parser existed, I
> > would consider using it. Information about whether a particular
> > option should/shouldn't go to restart command could be placed in a
> >    separate array near `entries'.
> 
> Maybe the other way round:
> put our command line options with all their information into a
> array of a struct which can be used by both, the GOptionParser (read:
> the values of the array are read and put into a new GOptionEntry array
> for the GOptionParser) and the SM code reads our custom array as well.
> Not sure whether this works, I didn't really have a look at the code.
> 

I think this won't work. GLib functions require `entries' to be an
array of GOptionEntry's. We can't put structs of a different size there.

> 
> 
> >  * Should I write a plugin? If not, should I extract the code into
> 
> Since we need to have a SM specific option in Geany itself anyways, I
> think it isn't worth moving the code into a plugin.

OK.

> Another problem
> would probably be the restart handling within a plugin since plugins
> might be loaded to late for this. Though not sure.

Actually it is not important when the plugin is loaded and unloaded.
libSM support can be properly initialized/deinitialized dynamically.

When we get to `main_finalize', all SM-related things are already done.
So unloading the plugin among the others in `main_finalize' is also safe.

> 
> And Nick already put your code into a branch in SVN, I consider this
> that we agree here :).
> 

I agree that a separate branch is needed. SM code can be polished
there, and SM-related commits will be in one place.

Best regards,
Eugene.