Merge lp://qastaging/~mkevac/gtg/578444 into lp://qastaging/~gtg/gtg/old-trunk

That certainty solves the problem, but we are trying to keep the UI-related classes free of "core" code as much as possible, and building the config object should go in core.
I know that most of our classes do not respect that, but we have two GSoC projects which will solve that problem (one involves separating GTG into a server-client software -like gwibber has done, and the other will create a new web interface for GTG).

To give you an indication about where that code should go, I've found where the part of the config object about plugin is built: in Manager.quit() GTG/viewmanager/manager.py.

I think that Manager.quit() is never called. That would be a good explanation for this bug.
Could you confirm that, Marko? If so, please modify GTG/gtg.py to call that function: it should be a one liner, but I'd like to have this merge with your name since you made me realize which was the (probable, for now) issue.

On Tue, Jun 1, 2010 at 1:05 PM, Luca Invernizzi <email address hidden> wrote:
> To give you an indication about where that code should go, I've found where the part of the config object about plugin is built: in Manager.quit() GTG/viewmanager/manager.py.
>
> I think that Manager.quit() is never called. That would be a good explanation for this bug.
> Could you confirm that, Marko? If so, please modify GTG/gtg.py to call that function: it should be a one liner, but I'd like to have this merge with your name since you made me realize which was the (probable, for now) issue.

It is called. To be more specific, it is called when main window is
closed (application is closed).
But it is not good to save configuration only when user exits the
application. Config should be saved right after some of configuration
parameters in changed.

I don't really understand why config should be "built". It should be
only read(), used and saved(). All "building" must be hidden in
Config() class.

So I don't see any way how some GUI that messes with configuration
could save that configuration in any way other than calling save() on
Config object.

I am not fond of my fix too. Primarily because "building" is done in
two places. In Preference class and in Manager class. But I am sure
that saving() must be available everywhere.

--
A. Because it breaks the logical sequence of discussion
Q. Why is top posting bad?

You're right, it is called. However, it is called only if the gtk_main_quit signal is intercepted by the TaskBrowser, which happens only if no abort or term signal is intercepted.
I think the correct solution to this bug should be some like this (I've made a merge request to show you the diff):
https://code.edge.launchpad.net/~gtg-user/gtg/bugfix-578444/+merge/26478

Secondly, I don't see why the plugins-related config is saved in the ViewManager, but that is another refactoring to be done.

On Tue, Jun 1, 2010 at 1:50 PM, Luca Invernizzi <email address hidden> wrote:
> You're right, it is called. However, it is called only if the gtk_main_quit signal is intercepted by the TaskBrowser, which happens only if no abort or term signal is intercepted.
> I think the correct solution to this bug should be some like this (I've made a merge request to show you the diff):
> https://code.edge.launchpad.net/~gtg-user/gtg/bugfix-578444/+merge/26478

There is no need for this, because interception of SIGTERM and SIGABRT
works as it should and config is saved when these signals are sent.

You can try this:
1. ./scripts/debug.sh
2. Enable some plugin
3. ps -eF | grep gtg
4. kill -15 <pid>
5. ./scripts/debug.sh

You will see that plugin is enables as you expect it to be.

Problem is with SIGKILL signal. There is no way to intercept SIGKILL.
So only way to fix bug is to save config _before_ SIGKILL happens.

--
A. Because it breaks the logical sequence of discussion
Q. Why is top posting bad?

That would certainty cover saving the plugins list, however that does not solve the broader scope of saving everything we should save on shut down. SIGKILL should not hit GTG.

GTG developers, I'd like to have your opinion on this one.

On Tue, Jun 1, 2010 at 2:17 PM, Luca Invernizzi <email address hidden> wrote:
> That would certainty cover saving the plugins list, however that does not solve the broader scope of saving everything we should save on shut down. SIGKILL should not hit GTG.

Sure!

I think best way will be using config object everywhere.

Right now, when user enables some plugin, it is enabled in
plugin-manager object. That is not right.
When enabling plugin, first change config.conf_dict parameter, than
use plugin-manager class to really enable it.

Same must be true for everything else (window coordinates, opened
windows, etc). Browser config is also separated from main config right
now.

Saving must be called right after config is changed.

--
A. Because it breaks the logical sequence of discussion
Q. Why is top posting bad?

On Tue, Jun 1, 2010 at 2:17 PM, Luca Invernizzi <email address hidden> wrote:
> GTG developers, I'd like to have your opinion on this one.

Anything new?

--
A. Because it breaks the logical sequence of discussion
Q. Why is top posting bad?

Ok. You've convinced me. Patch merged.

Ehi, Marko, thanks for your work!

Marko, if you are interested, I can add you to the GTG contributors (https://edge.launchpad.net/~gtg-contributors) team and mailing list. It's the developers mailing list, where the interesting discussions on the future of GTG take place :)

On Thu, Jun 3, 2010 at 2:43 AM, Luca Invernizzi <email address hidden> wrote:
> Marko, if you are interested, I can add you to the GTG contributors (https://edge.launchpad.net/~gtg-contributors) team and mailing list. It's the developers mailing list, where the interesting discussions on the future of GTG take place :)

I am.

About last patch. It's pity that other developers didn't participated
in discussion. Changes that I made fix the bug, yes, but I am not
totally satisfied. All of config parameters should be saved right
away, not only active plugin list.

What should I do? Should I start conversation in this mailing list? Or
should I open bug and propose changes right away? These changes will
touch a lot of things I expect.

P.S. Do bugs in lp have "closed" status, or "patch commited" is "last" one?

--
A. Because it breaks the logical sequence of discussion
Q. Why is top posting bad?

If you need to bring something to all developers attention yes, we should use the mailing list. However, please consider that we are a small team and we have a lot of stuff going on (and jobs), so answers cannot be very fast. After all, this merge request was active for just a few days.
Answers will come eventually :)

Bugs are marked "fix committed" when the bug is fixed in Trunk. When we make the next GTG release (0.25 or 0.3 will be the next), those are marked "fix released", which is the "closed" bug status

You are in the team, but you have to subscribe yourself in the mailing list from https://edge.launchpad.net/~gtg-contributors