Created attachment 27501 [details]
glade-session.h

Created attachment 27502 [details]
glade-session.c

The attached file is a first cut implementation: it stores the position of the
various windows in a ~./glade-session file. It needs a bit more work, but I'd
like  to hear if this goes in the proper direction or if this approach sucks
:-)

Among other things that need doing is a bit more error-checking, support for
the widget-tree and clipboard (since they are not created by default) and
probably other things.

For now it only stores window position, other possible addition are window
dimension and if the tools (for instance the palette) are shown or not.

Created attachment 27503 [details] [review]
glue patch

I think the general approach is fine.

Do we really need an XML writer? Is there any advantage over fprintf?

I really hate . files in $HOME. I wish there was a standard subdirectory to
put these files. Maybe $HOME/.gnome2.

I'm not partcularly attached to use xml, I just copied what epiphany uses :-) I
suppose that using it makes parsing easier and a little more robust. I also took
a look at what gimp does and, while similar in the approach, not using libxml
makes things much more complicated.

.gnome2 exists but afaic is used to save gnome-config stuff... beside glade3 is
not a gnome app.

Hi,

Paolo, it's also not clear to me the advantage of using a XML writer.
We can still save it in correct xml using a fprintf. You will parse it the same
way as you do now, but instead of your long write_window_geometry, you will only
do a:

gtk_window_get_size (window, &width, &height);
gtk_window_get_position (window, &x, &y);
fprintf(file, "<window id=\"%d\" x=\"%d\" y=\"%d\" width=\"%d\"
height=\"%d\"/>\n", id, x, y, width, height);

Cheers,

fine with me... as I said above that function is cut&pasted from epiphany :-)

Created attachment 27629 [details]
first try at enhancing Paolo's patch

So, this is my first pick at trying to improve the patch/fix the issues raised.
I think the first step (doing some more error checking and not using the
xmlwriter) are already done. Next step is dealing with the other two windows.

I've made a change: use a ~/.glade/session file instead of a ~/.glade-session.
That's because I, too, don't like stuff on ~/.* namespace and, thus, I think
this is a sane way of having our needs satisfied and prepare the path for new
potencial aditions of RC files.

Also, I enabled saving height/width as I think saving window geometry is
desirable. Comments on the patch? Is it the approach you wish to go with?

Cool! Thanks for doing this Gustavo.

Minor comments:
#include xmlwriter can probably go now
feel free to add yourself to the authors list of the file :)

also I did not save the window dimension because I'm not sure it make sense for
the palette...

Joaquin, Damon: if you are ok with this I think we can commit it and improve
incrementally from cvs

Created attachment 27643 [details] [review]
proposed patch

Heya, Thanks Paolo =)

I have done some more hacking and I got all the functionality working now for
all windows. This is a proposed patch but it modifies lots more on
glade-project-window.c.

It creates both widget_tree and clipboard_view at 'boot' time. I modified all
the gpw_create_* stuff for the editor, palette, etc to not enable the menu
entry at 'boot' time, too, so that I could choose to show the window or not
when reading the xml configuration.

Especially, I modified the widget_tree creation function to be consistent with
the other ones which set gpw-><var>widget</var> (if you understand me =))
instead of returning it (damn, it took me some time to figure out this segfault
=D).

Well, that's basicaly it. What do you think?

Hi gustavo, I just took a quick look at your last patch, but here are some comments:

while I like the fact that you made the tree view creation similar to the other
widgets (btw, clipbard view may like that treatment too ;). I start to think
things are becoming a little hackish here: I don't like the fact that we are
exposing some of the internals of the project-window (especially accessing the
item-factory... at that point I would prefer exporting all the gpw_show_* functions)

The first alternative that comes to my mind is the following: create a
glade_session struct in glade-session.h and put one of those in gpw->session.
Such struct would contain x, y, width, height and to_show for each window.
session_restore() would be called before creating the widgets so that x,y etc
are available when creating the widgets. If the session file is not there or if
an error occurs the struct would be filled with defaults values.

What do you (and Joaquin and Damon) think?


Also I don't like the hack to encode the visibility into the return value of
restore_window_geometry(): just add another arg to the fuction or something like
that.

Last couple of nits:

- we prefer 
/* blah blah
 * blah
 */
for comments

- you added some extra whitespaces in some of the indentation (if you look at
the patch with the browser in bugzilla, you'll notice them)

Hi!

Thanks for the comments and hints on code consistency! I'm sorry for that, I
would have checked other source files but I was in a hurry after I finished the
patch.

About the 'hackish' stuff. I agree. I just didn't want to mess a lot with the
code, but I agree there's a need for rethinking some parts of it. I'm not sure
this bug report is the best place to describe this stuff but, yes, the first big
problem I had was with gpw_show_* and gpw_hide_* stuff being unavailable to me.
So I ended up duplicating the code by exposing the menu stuff in glade-session.

I believe your solution looks good, but I think we could use this oportunity to
also go forward on code 'widgetization'. It's currently still impossible to use
the glade palette, for example, in a program without having a
glade_project_window (because the functions that 'generate' the UI require a gpw
and the lower level functions are not exposed). The same probably goes for
widget_tree.

I thought about adding a glade-widget-tree.{c,h} and widgetizing it, and maybe
provide the show and hide as methods for those widgets (editor, palette, etc)
but as I said, I would prefer not to mess a lot with the code without talking to
you people first.

Actualy. Thinking a bit more, adding those methods to a GladeWidgetTree makes no
sense. Do you think making gpw_show_* available to other modules as glade_show_*
would make sense?

Also, do you think this discussion should be moved to the list?

*** Bug 372323 has been marked as a duplicate of this bug. ***

*** Bug 400847 has been marked as a duplicate of this bug. ***

Marking this patch obsolete since it was written over 2 years
ago and the code has changed significantly.

*** Bug 404494 has been marked as a duplicate of this bug. ***

This is implemented in Glade currently at 2 levels.

  - glade_app_config_load/save() takes care of internal libgladeui
    session data (currently I think thats just palette formats although
    used to contain contextual property help mode)

  - glade-window.c: Also saves session data regarding the size, position
    and states of the main window and editor dock windows.

Other session data related issues should be treated on a case by
case basis by now, closing this bug.