Hi all.
The attached patches implement X session management protocol (XSMP) support via libSM library.
Why XSMP?
When a user logs out with some instances of Geany running, XSMP support will allow those instances to safely save their configuration and gracefully ask the user about unsaved documents. The logout process will halt until Geany instances report that they are ready for shutdown.
When the user logs in, all Geany instances will be restored, with the same documents open. XSMP allows us to set a restart command for each Geany instance, so I just remember open documents by passing their file paths via command-line arguments.
Why libSM?
This library is as lightweight as possible. AFAIK, it is written by the authors of Xorg. It is also very popular, used in claws-mail and required by GNOME.
Patches:
[1.autotools.patch]
Building with X session management support if libSM library is present. Is the library is not installed or ./configure was run with `--enable-libsm=no', XSMP support is disabled.
I tested three cases: * libSM installed; * libSM installed, --enable-libsm=no; * libSM is not installed.
[2.waf.patch]
The same thing, but using waf instead of autotools. It is possible to disable XSMP support by passing `--disable-libsm' to `./waf congigure'.
Again, I tested three cases: * libSM installed; * libSM installed, --disable-libsm supplied; * libSM is not installed.
[3.refactor.patch]
This is probably the most doubtful part. In order to support XSMP properly, I need:
1) a non-destructive function to completely save Geany's state;
2) a function to quit Geany without saving anything and interacting with user.
I found `main_quit' function to satisfy the (2) requirements. But there was no function to satisfy (1), so I created a new one called `main_save' from `quit_app' function located in [callbacks.c].
I also had to
* add `force' argument to `document_close_all' function to be used when you do not want the function to ask about unsaved documents;
* create a trivial `project_save' function; add `save_config' argument to `project_close' function so that I can set it to FALSE when I already called `project_save'.
I wanted to reuse existing static `check_no_unsaved' function located in [callbacks.c], so I created new `document_any_unsaved' function from it.
I renamed `main_quit' function to `main_finalize' in order to point out that it does not interact with user and does not save anything (yes, it's doubtful and I'll change it back if you wish).
As far as I understand, none of the changed functions are exported for plugins, so the changes do not break API or ABI compatibility.
[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.
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.
Problems:
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.
If all geany-devel subscribers agree, we should ask geany-users. If they agree too, anyone can write the necessary code, and I will change the handling of `--no-session' in my [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.
TODO:
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).
There is a little problem with this task: if I write all command-line handling code in my `sm_set_command_props' function, there will be code duplication between `sm_set_command_props' implementation and the array of GOptionEntry. Every time when a new command-line option is added, `sm_set_command_props' will have to be changed, particularly:
* the name of the variable, where the option's value is stored, will have to be duplicated there;
* the handling of the option will depend on option's type (int, string, etc.), which is directly corresponds to the type specified in the `entries' array.
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 write a plugin?
Yes, it is possible to write a plugin instead of building XSMP support straight into Geany. The plugin would require:
* some data when initializing (argv[0] and the value of `--libsm-client-id' command-line option);
* access to `cl_options' struct to set restart command properly;
* access to `main_save' and `main_finalize' functions in order to control Geany.
A bad thing about this is that we'll still have to hardcode `--libsm-client-id' command-line option in Geany's source code, even with XSMP support residing in a plugin.
Summary of questions
* Are there anyone who thinks that `--new-instance's should save Geany session?
* Are there any "reverse" parser of GOptionEntries?
* Should I write a plugin? If not, should I extract the code into separate source code files?
Hope you find it useful. Any thoughts?
Best regards, Eugene.
On Tue, 24 Nov 2009 16:00:06 +0300 Eugene Arshinov earshinov@gmail.com wrote:
The attached patches implement X session management protocol (XSMP) support via libSM library.
Thanks for the patch. I haven't had time to look at it yet but the basic idea sounds OK to me. From only a quick read through your email I'm not sure how this would work with Geany's existing session restore feature though.
Probably as a first step we can (all?) agree on is to add support for just starting the first instance when the user logs in and using Geany's session restore to load files and set cursor position, filetype, etc.
What does everyone think about this?
Regards, Nick
Hi,
2009/12/3 Nick Treleaven nick.treleaven@btinternet.com:
On Tue, 24 Nov 2009 16:00:06 +0300 Eugene Arshinov earshinov@gmail.com wrote:
The attached patches implement X session management protocol (XSMP) support via libSM library.
Thanks for the patch. I haven't had time to look at it yet but the basic idea sounds OK to me. From only a quick read through your email I'm not sure how this would work with Geany's existing session restore feature though.
Probably as a first step we can (all?) agree on is to add support for just starting the first instance when the user logs in and using Geany's session restore to load files and set cursor position, filetype, etc.
What does everyone think about this?
I like the idea of Geany re-starting itself on login, but agree that its better to only have one set of session restore code in Geany used both for automatic re-start and for restoring sessions when manually started.
Cheers Lex
Regards, Nick _______________________________________________ Geany-devel mailing list Geany-devel@uvena.de http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
On Tue, 24 Nov 2009 16:00:06 +0300, Eugene wrote:
Hi,
sorry, I didn't manage to read and reply to your mail before. Even though I'm really happy you invested so much efforts into this. Thanks!
(Everything of your original mail I snipped out is ok for me and I spare us the 'yes, I agree' sentences :D)
[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.
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.
- 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.
[4.implementation.patch].
Untested functionality:
- 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.
- 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.
- 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. Another problem would probably be the restart handling within a plugin since plugins might be loaded to late for this. Though not sure.
And Nick already put your code into a branch in SVN, I consider this that we agree here :).
Regards, Enrico
Hi Enrico.
On Tue, 8 Dec 2009 21:06:10 +0100% Enrico Tröger enrico.troeger@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.
- 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:
- 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.
- 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.
Hi.
Here are the patches for SM branch I promised to send. Proposed commit message and notes (if any) are given below.
On my way I noticed some minor bugs. The patches which fix those bugs have "fix" in their names.
Now I'm going to write a reverse parser for GOptionEntry. To proceed, I need to know where I should put the code. Should I (a) modify utils.{c,h} files or (b) make my own optentries.{c,h}? I want to declare new GeanyOptionEntryAux struct containing one gboolean and implement two functions:
* a function to reverse-parse a single GOptionEntry;
* a function taking an array of GOptionEntry and an array of GeanyOptionEntryAux and returning a GArray filled like argv.
Personally, I'd prefer (b).
[sm.1.files.patch] “Extract SM-related code into separate sm.{c,h} files, make some refactoring, and write code comments for Doxygen.”
Note: if you consider some Doxygen comments excessive, just drop them.
[sm.2.sm_finalize.patch] “Add sm_finalize().”
[sm.3.fix-restart-command.patch] “Fix restart command.”
[sm.4.fix-interaction.patch] “Handle "Interact" message properly.”
[sm.5.noinstance-behaviour.patch] “Make --new-instance imply --no-session.”
[sm.6.nosession.patch] “Handle --no-session properly.
The previous commit allows us to change --no-session command line option handling back to normal. The issue was described in a FIXME section inside src/sm.c, the section is now deleted.”
[sm.7.fix-filenames.patch] “Use `GeanyDocument.file_name's instead of `GeanyDocument.real_path's in restart command like Geany session management facilities do.”
Best regards, Eugene.
Sorry, here are the patches themselves :)