[Bluez-devel] [PATCH] Simplify about dialog code

[Bluez-devel] [PATCH] Simplify about dialog code

[Jaap A. Haitsma]

[-- Attachment #1: Type: text/plain, Size: 398 bytes --]

Hi Marcel,

Before you're holidays I sent you a patch which simplifies the about
dialog code. We went on discussing it but didn't come to a conclusion,
I think

In my opinion it's a good patch because:

1) The patch removes about 20 function calls
2) You don't need  to hookup signals to the about dialog
3) Eog, cheese, gnome-power-manager, nautilus, gedit to name a few all
do it this way.

Jaap

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: bluez-gnome-about-dialog.patch --]
[-- Type: text/x-patch; name=bluez-gnome-about-dialog.patch, Size: 5146 bytes --]

Index: analyzer/dialog.c
===================================================================
RCS file: /cvsroot/bluez/gnome/analyzer/dialog.c,v
retrieving revision 1.24
diff -u -r1.24 dialog.c
--- analyzer/dialog.c	10 Feb 2008 20:35:22 -0000	1.24
+++ analyzer/dialog.c	10 Feb 2008 22:21:46 -0000
@@ -32,11 +32,6 @@
 #include "dialog.h"
 #include "discover.h"
 
-static const gchar *authors[] = {
-	"Marcel Holtmann <marcel@holtmann.org>",
-	NULL
-};
-
 void show_error_dialog(const gchar *message)
 {
 	GtkWidget *dialog;
@@ -93,43 +88,24 @@
 {
 	GtkWidget *dialog;
 
-	dialog = gtk_about_dialog_new();
-
-	gtk_window_set_position(GTK_WINDOW(dialog),
-					GTK_WIN_POS_CENTER_ON_PARENT);
-
-	gtk_about_dialog_set_name(GTK_ABOUT_DIALOG(dialog),
-						_("Bluetooth Analyzer"));
-
-	gtk_about_dialog_set_version(GTK_ABOUT_DIALOG(dialog), VERSION);
-
-	gtk_about_dialog_set_copyright(GTK_ABOUT_DIALOG(dialog),
-			"Copyright \xc2\xa9 2005-2008 Marcel Holtmann");
-
-	gtk_about_dialog_set_comments(GTK_ABOUT_DIALOG(dialog),
-				_("Bluetooth protocol analyzer"));
-
-	gtk_about_dialog_set_logo_icon_name(GTK_ABOUT_DIALOG(dialog),
-							"bluetooth");
+	static const gchar *authors[] = {
+		"Marcel Holtmann <marcel@holtmann.org>",
+		NULL
+	};
 
 	gtk_about_dialog_set_url_hook(about_url_hook, NULL, NULL);
-
 	gtk_about_dialog_set_email_hook(about_email_hook, NULL, NULL);
 
-	gtk_about_dialog_set_website(GTK_ABOUT_DIALOG(dialog),
-						"http://www.bluez.org");
-
-	gtk_about_dialog_set_website_label(GTK_ABOUT_DIALOG(dialog),
-							"www.bluez.org");
-
-	gtk_about_dialog_set_authors(GTK_ABOUT_DIALOG(dialog), authors);
-
-	gtk_about_dialog_set_translator_credits(GTK_ABOUT_DIALOG(dialog),
-						_("translator-credits"));
-
-	gtk_dialog_run(GTK_DIALOG(dialog));
-
-	gtk_widget_destroy(dialog);
+	gtk_show_about_dialog(NULL,
+			      "version", VERSION,
+			      "copyright", "Copyright \xc2\xa9 2005-2008 Marcel Holtmann",
+			      "comments", _("Bluetooth protocol analyzer"),
+			      "authors", authors,
+			      "translator-credits", _("translator-credits"),
+			      "website", "http://www.bluez.org",
+			      "website-label", "www.bluez.org",
+			      "logo-icon-name", "bluetooth",
+			      NULL);
 }
 
 static void add_filters(GtkFileChooser *chooser)
Index: analyzer/main.c
===================================================================
RCS file: /cvsroot/bluez/gnome/analyzer/main.c,v
retrieving revision 1.38
diff -u -r1.38 main.c
--- analyzer/main.c	10 Feb 2008 17:46:50 -0000	1.38
+++ analyzer/main.c	10 Feb 2008 22:21:48 -0000
@@ -1029,6 +1029,8 @@
 		gtk_exit(1);
 	}
 
+ 	g_set_application_name(_("Bluetooth Analyzer"));
+
 	gtk_window_set_default_icon_name("bluetooth");
 
 	create_window();
Index: applet/main.c
===================================================================
RCS file: /cvsroot/bluez/gnome/applet/main.c,v
retrieving revision 1.108
diff -u -r1.108 main.c
--- applet/main.c	10 Feb 2008 20:35:22 -0000	1.108
+++ applet/main.c	10 Feb 2008 22:21:48 -0000
@@ -513,46 +513,20 @@
 		"Bastien Nocera <hadess@hadess.net>",
 		NULL
 	};
-	GtkWidget *dialog;
-
-	dialog = gtk_about_dialog_new();
-
-	gtk_about_dialog_set_name(GTK_ABOUT_DIALOG(dialog),
-						_("Bluetooth Applet"));
-
-	gtk_about_dialog_set_version(GTK_ABOUT_DIALOG(dialog), VERSION);
-
-	gtk_about_dialog_set_copyright(GTK_ABOUT_DIALOG(dialog),
-			"Copyright \xc2\xa9 2005-2008 Marcel Holtmann");
-
-	gtk_about_dialog_set_comments(GTK_ABOUT_DIALOG(dialog),
-			_("A Bluetooth manager for the GNOME desktop"));
-
-	gtk_about_dialog_set_logo_icon_name(GTK_ABOUT_DIALOG(dialog),
-							"bluetooth");
-
-	gtk_about_dialog_set_url_hook(about_url_hook, NULL, NULL);
-
-	gtk_about_dialog_set_email_hook(about_email_hook, NULL, NULL);
-
-	gtk_about_dialog_set_website(GTK_ABOUT_DIALOG(dialog),
-						"http://www.bluez.org");
-
-	gtk_about_dialog_set_website_label(GTK_ABOUT_DIALOG(dialog),
-							"www.bluez.org");
-
-	gtk_about_dialog_set_authors(GTK_ABOUT_DIALOG(dialog), authors);
-
-	gtk_about_dialog_set_translator_credits(GTK_ABOUT_DIALOG(dialog),
-						_("translator-credits"));
-
-	g_signal_connect(dialog, "close",
-				G_CALLBACK(close_callback), NULL);
-
-	g_signal_connect(dialog, "response",
-				G_CALLBACK(close_callback), NULL);
-
-	gtk_widget_show_all(dialog);
+ 
+ 	gtk_about_dialog_set_url_hook(about_url_hook, NULL, NULL);
+ 	gtk_about_dialog_set_email_hook(about_email_hook, NULL, NULL);
+ 
+	gtk_show_about_dialog(NULL,
+			      "version", VERSION,
+			      "copyright", "Copyright \xc2\xa9 2005-2008 Marcel Holtmann",
+			      "comments", _("A Bluetooth manager for the GNOME desktop"),
+			      "authors", authors,
+			      "translator-credits", _("translator-credits"),
+			      "website", "http://www.bluez.org",
+			      "website-label", "www.bluez.org",
+			      "logo-icon-name", "bluetooth",
+			      NULL);
 }
 
 static void settings_callback(GObject *widget, gpointer user_data)
@@ -849,6 +823,8 @@
 	if (instance == NULL)
 		gtk_exit(0);
 
+	g_set_application_name (_("Bluetooth Applet"));
+
 	gtk_window_set_default_icon_name("bluetooth");
 
 	conn = dbus_g_bus_get(DBUS_BUS_SYSTEM, &error);

[-- Attachment #3: Type: text/plain, Size: 228 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[-- Attachment #4: Type: text/plain, Size: 164 bytes --]

_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] Simplify about dialog code

[Marcel Holtmann]

Hi Jaap,

> Before you're holidays I sent you a patch which simplifies the about
> dialog code. We went on discussing it but didn't come to a conclusion,
> I think

I am not going on holidays. At least not that I know of :)

> In my opinion it's a good patch because:
> 
> 1) The patch removes about 20 function calls
> 2) You don't need  to hookup signals to the about dialog
> 3) Eog, cheese, gnome-power-manager, nautilus, gedit to name a few all
> do it this way.

Still not convinced.

Regards

Marcel



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] Simplify about dialog code

[Bastien Nocera]

On Mon, 2008-02-11 at 02:32 +0100, Marcel Holtmann wrote:
> Hi Jaap,
> 
> > Before you're holidays I sent you a patch which simplifies the about
> > dialog code. We went on discussing it but didn't come to a conclusion,
> > I think
> 
> I am not going on holidays. At least not that I know of :)
> 
> > In my opinion it's a good patch because:
> > 
> > 1) The patch removes about 20 function calls
> > 2) You don't need  to hookup signals to the about dialog
> > 3) Eog, cheese, gnome-power-manager, nautilus, gedit to name a few all
> > do it this way.
> 
> Still not convinced.

Using gtk_dialog_run() means that you're running the main loop from the
about box code. Fixing this would add code. The patch Jaap sent removes
a lot of code, and avoids things like double-about boxes, or crappy
backtraces that show the about box callback function :)


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] Simplify about dialog code

[Jaap A. Haitsma]

On Feb 11, 2008 2:39 AM, Bastien Nocera <hadess@hadess.net> wrote:
>
> On Mon, 2008-02-11 at 02:32 +0100, Marcel Holtmann wrote:
> > Hi Jaap,
> >
> > > Before you're holidays I sent you a patch which simplifies the about
> > > dialog code. We went on discussing it but didn't come to a conclusion,
> > > I think
> >
> > I am not going on holidays. At least not that I know of :)
> >
> > > In my opinion it's a good patch because:
> > >
> > > 1) The patch removes about 20 function calls
> > > 2) You don't need  to hookup signals to the about dialog
> > > 3) Eog, cheese, gnome-power-manager, nautilus, gedit to name a few all
> > > do it this way.
> >
> > Still not convinced.
>
> Using gtk_dialog_run() means that you're running the main loop from the
> about box code. Fixing this would add code. The patch Jaap sent removes
> a lot of code, and avoids things like double-about boxes, or crappy
> backtraces that show the about box callback function :)
>
Marcel,

The patch even fixes bugs. So that should be enough reason to apply it, right?
Otherwise I'm interested in hearing your arguments against it.

Regards,

Jaap

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] Simplify about dialog code

[Marcel Holtmann]

Hi Bastien,

> > > Before you're holidays I sent you a patch which simplifies the about
> > > dialog code. We went on discussing it but didn't come to a conclusion,
> > > I think
> > 
> > I am not going on holidays. At least not that I know of :)
> > 
> > > In my opinion it's a good patch because:
> > > 
> > > 1) The patch removes about 20 function calls
> > > 2) You don't need  to hookup signals to the about dialog
> > > 3) Eog, cheese, gnome-power-manager, nautilus, gedit to name a few all
> > > do it this way.
> > 
> > Still not convinced.
> 
> Using gtk_dialog_run() means that you're running the main loop from the
> about box code. Fixing this would add code. The patch Jaap sent removes
> a lot of code, and avoids things like double-about boxes, or crappy
> backtraces that show the about box callback function :)

I have really no idea what you are talking about. How does this change
do all this magic?

And what crappy backtraces?

Regards

Marcel



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] Simplify about dialog code

[Marcel Holtmann]

Hi Jaap,

> > > > Before you're holidays I sent you a patch which simplifies the about
> > > > dialog code. We went on discussing it but didn't come to a conclusion,
> > > > I think
> > >
> > > I am not going on holidays. At least not that I know of :)
> > >
> > > > In my opinion it's a good patch because:
> > > >
> > > > 1) The patch removes about 20 function calls
> > > > 2) You don't need  to hookup signals to the about dialog
> > > > 3) Eog, cheese, gnome-power-manager, nautilus, gedit to name a few all
> > > > do it this way.
> > >
> > > Still not convinced.
> >
> > Using gtk_dialog_run() means that you're running the main loop from the
> > about box code. Fixing this would add code. The patch Jaap sent removes
> > a lot of code, and avoids things like double-about boxes, or crappy
> > backtraces that show the about box callback function :)
>
> The patch even fixes bugs. So that should be enough reason to apply it, right?
> Otherwise I'm interested in hearing your arguments against it.

this is the first time that I hear it actually fixes bugs (maybe I
wasn't listen earlier), but actually you have to explain it. Telling me
that it fixes bugs (that I am not aware of) isn't enough. Show me the
bug and how it fixes it and it is highly likely that I simply go ahead
and apply your patch.

Btw. I did a new release and forgot to check if the patch would still
apply. If not, care to redo it.

Regards

Marcel



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] Simplify about dialog code

[Bastien Nocera]

On Tue, 2008-02-12 at 00:23 +0100, Marcel Holtmann wrote:
> Hi Bastien,
> 
> > > > Before you're holidays I sent you a patch which simplifies the about
> > > > dialog code. We went on discussing it but didn't come to a conclusion,
> > > > I think
> > > 
> > > I am not going on holidays. At least not that I know of :)
> > > 
> > > > In my opinion it's a good patch because:
> > > > 
> > > > 1) The patch removes about 20 function calls
> > > > 2) You don't need  to hookup signals to the about dialog
> > > > 3) Eog, cheese, gnome-power-manager, nautilus, gedit to name a few all
> > > > do it this way.
> > > 
> > > Still not convinced.
> > 
> > Using gtk_dialog_run() means that you're running the main loop from the
> > about box code. Fixing this would add code. The patch Jaap sent removes
> > a lot of code, and avoids things like double-about boxes, or crappy
> > backtraces that show the about box callback function :)
> 
> I have really no idea what you are talking about. How does this change
> do all this magic?

The change binds the GtkAbout to the application so GTK+ keeps track of
the about box existing or not, and not us. So it avoids multiple about
boxes being displayed.

> And what crappy backtraces?

Crappy backtrace are the ones run from within the gtk_run_dialog() in
the about dialog callback. When yoy show the about box, and don't
dismiss it, your application is running its main loop from your
gtk_dialog_run() call. If the application crashes, the backtrace will be
from your gtk_dialog_run() call, not from the gtk_main() in main.

It's a simple fix that rids your own code of the book-keeping for this
dialogue.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] Simplify about dialog code

[Marcel Holtmann]

Hi Bastien,

> > > > > Before you're holidays I sent you a patch which simplifies the about
> > > > > dialog code. We went on discussing it but didn't come to a conclusion,
> > > > > I think
> > > > 
> > > > I am not going on holidays. At least not that I know of :)
> > > > 
> > > > > In my opinion it's a good patch because:
> > > > > 
> > > > > 1) The patch removes about 20 function calls
> > > > > 2) You don't need  to hookup signals to the about dialog
> > > > > 3) Eog, cheese, gnome-power-manager, nautilus, gedit to name a few all
> > > > > do it this way.
> > > > 
> > > > Still not convinced.
> > > 
> > > Using gtk_dialog_run() means that you're running the main loop from the
> > > about box code. Fixing this would add code. The patch Jaap sent removes
> > > a lot of code, and avoids things like double-about boxes, or crappy
> > > backtraces that show the about box callback function :)
> > 
> > I have really no idea what you are talking about. How does this change
> > do all this magic?
> 
> The change binds the GtkAbout to the application so GTK+ keeps track of
> the about box existing or not, and not us. So it avoids multiple about
> boxes being displayed.

never realized that. You should have pointed that out earlier.

> > And what crappy backtraces?
> 
> Crappy backtrace are the ones run from within the gtk_run_dialog() in
> the about dialog callback. When yoy show the about box, and don't
> dismiss it, your application is running its main loop from your
> gtk_dialog_run() call. If the application crashes, the backtrace will be
> from your gtk_dialog_run() call, not from the gtk_main() in main.
> 
> It's a simple fix that rids your own code of the book-keeping for this
> dialogue.

I applied the change to the CVS now.

Regards

Marcel



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel