Created attachment 92966 [details] [review]
patch

Any comments are welcome.

Cool.  Thanks for working on this.  I'd like to get this in early in the next development cycle.

I haven't had time to play with GtkBuilder yet so the only comment I have so far on the patch is that the indentation seems to be inconsistent with the style of the files.  Hopefully that should be easy to fix (the emacs mode thingy should make that simple).

If you can fix that up we'll get this in right after we branch.  Thanks!

Created attachment 94501 [details] [review]
updated patch

I eliminated the indentation inconsistency.

Created attachment 94506 [details] [review]
New patch

This one should work again.

Is there a good gui tool to use to work with gtkbuilder files?

Well, isn't glade 3 appropriate ?
Oh, and one remark about the patch: it should check for (ui == NULL) before using it (src/gnome-screensaver-preferences.c and src/gs-lock-plug.c).

Status on this?

Created attachment 105291 [details] [review]
Updated patch, should compile fine against svn

There's still some indentation issues. Oh, and while you're at it, as I told you previously, xml loading should be checked before it's used, unless you consided it's a separate problem that you're patch is not supposed to fix.

This code:
==========
xml = gtk_builder_new ();
gtk_builder_add_from_file (xml, gtkbuilder, &error);
gtk_builder_set_translation_domain(xml, GETTEXT_PACKAGE);
 
if (xml == NULL) {
       /* Some error management */
       return FALSE;
}


Would be better written:
========================
xml = gtk_builder_new ();
if (xml == NULL) {
       /* Some error management */
       return FALSE;
}

gtk_builder_add_from_file (xml, gtkbuilder, &error);
gtk_builder_set_translation_domain (xml, GETTEXT_PACKAGE);

And that's not needed, and would break with the single-include GTK+
-#include <glade/glade-xml.h>
+#include <gtk/gtkbuilder.h>

Yep, should be :
-#include <glade/glade-xml.h>
+#include <gtk/gtk.h>

But at the time the patch was written, I'm not sure this was an official policy :-)

Two notes:
 * some modules, for instance nautilus, prepends "[type: gettext/glade]" to
   GtkBuilder file in POTFILES.in (dunno if this is required, suggested or else)
 * why not direclty jump to GTK+ 2.16 and use new GtkEntry features such as 
   CapsLock warning and "gtk-stock-authenticate" as primary icon
   (see this[1])?

> Is there a good gui tool to use to work with gtkbuilder files?

Latest glade3 supports all gtkbuilder feautures (except some advanced stuff not needed by gnome-screensaver) so IMHO there are no more issues to perform this switch in 2.26 release.

Of course this will break existing themes, so you need to add some good updating guide: "Pick up glade3 version 3.5.x. Open your theme. Save choosing GtkBuilder instead glade" (needs to be tested, of course).

BTW there is a theme creation guide on live.gnome.org or elsewhere?

[1] http://svn.gnome.org/viewvc/gtk%2B/trunk/tests/testentryicons.c

(In reply to comment #12)

>  * why not direclty jump to GTK+ 2.16 and use new GtkEntry features such as 
>    CapsLock warning and "gtk-stock-authenticate" as primary icon
>    (see this[1])?
> 

It seems that CapsLock warning is automatically added to GtkEntries using invisible characters.

ping.

Someone willing to come up with an updated patch?

Created attachment 137876 [details] [review]
Updated patch

Any comments?

Patch looks good but needs a couple of changes

1) Prepend "[type: gettext/glade]" to GtkBuilder file in POTFILES.in

2) Please take a look at indentation. gnome-screensaver uses identation of 8 spaces it doest not use tabs.

A couple of examples
-----
-static GladeXML       *xml = NULL;
+static GtkBuilder       *builder = NULL;
 static GSThemeManager *theme_manager = NULL;
-------
+        if (!gtk_builder_add_from_file (builder,filename ,&error)) {
+                g_warning ("Couldn't load builder file: %s", error->message);
+		g_error_free(error);
+                g_free (gtkbuilder);
                 return FALSE;
         }

------
+	if (!gtk_builder_add_from_file(builder, gtk_builder_file, &error))
+		{
+			g_warning("Couldn't load builder file: %s", error->message);
+			g_error_free(error);
+		}	
+        g_free (gtk_builder_file);
------ 
gnome-screensaver uses 
if (blah) {
        do_something ();
}
-----------

At the top of each sourcefile there is
/* -*- Mode: C; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 8 -*-

When you use an editor that supports that syntax such as vim or gedit it will make sure automatically that you have the right indentation

Committed with those cleanups (since I need the GtkBuilder conversion for something else)

Created attachment 138556 [details] [review]
Remove some unused files

I've reopened this bug because there are some unused files after the port

Shouldn't $(glade_DATA) be replaced by $(gtkbuilder_DATA) ?
Except of that, I suppose your patch is right. If noone complains, I suggest you commit it.

Should be fixed now.  Thanks.