Re: [Bluez-devel] [PATCH] Beautify about dialog

["Jaap A. Haitsma" <jaap@haitsma.org>]

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

On Dec 19, 2007 12:22 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Jaap,
>
> > > > > > > > > Attached patch does the following
> > > > > > > > >
> > > > > > > > > * about dialog code is simpler
> > > > > > > >
> > > > > > > > you can't use gtk_dialog_run for the applet's about dialog. It will
> > > > > > > > block and thus block all PIN requests. The current code is this way for
> > > > > > > > a reason.
> > > > > > >
> > > > > > > Didn't know that. I now use g_object_set which also reduces the amount
> > > > > > > of code considerably.
> > > > > >
> > > > > > You can still use gtk_show_about_dialog(). It removes the close
> > > > > > callback, and makes sure that only one dialogue shows up, and removes a
> > > > > > lot of boiler-plate code (it doesn't use a separate main loop like
> > > > > > gtk_dialog_run does). The gtk_dialog_run avoidance in the callbacks is
> > > > > > still needed.
> > > > >
> > > > > not sure why I haven't done it that way. Maybe it was not available when
> > > > > I wrote the initial version. However now the requirements are GTK 2.10
> > > > > or later. So if we can simplify code, I am all for it.
> > > > >
> > > > Attached two patches
> > > >
> > > > One uses gtk-show-about-dialog
> > >
> > > fix the coding style for this one. I can already see that it messes up
> > > the whitespaces. We use tabs and no whitespace in front of the ( in
> > > function declarations.
> >
> > Attached patch with coding style fixup attached
>
> I don't see the advantage of this patch. Especially with this ugly ifdef
> for the program-name bug, I decided to not apply it.

The "program-name" thing actually solved a bug in GTK+. See [1]
The ugly ifdef you don't have to do if you use g_set_application_name
[2]. Can you explain what's against against using
g_set_application_name? It seems a handy function to me. You call it
once in main and it's set correctly for error dialogs etc.

> Especially the
> license crap is not worth doing it. There is no need that the UI shows
> the license in the about dialog.

As a first time contributor to bluez-gnome I'm confused.

You request me to do all kind of changes,  which I do. I can
understand that because you don't want to spend your time fixing up
patches of others.
You agree with Bastien if it results in simplification its good.  You
ask me to fix the coding style. I do that and also don't use
g_set_application_name because you don't like it. Then you don't apply
it because you say you don't like ifdef and the license.

If you don't count the license code my patch replaces 13 functions
with just 1. Isn't that a simplification?

I'm interested in helping out with bluez-gnome because I think it's a
great app. However if the way this patch got treated is the normal way
I'm not sure if I will have the motivation to submit other patches

Anyway, licenses are shown in many GTK programs. To name a few GIMP,
gedit, nautilus, eog, gnome-terminal, gnome-power-manager, all
gnome-games, gcalculator

But in case you don't want the license. I attached two versions of the
patch. They both use g_set_application_name to remove the #if but one
is with the license and the other isn't.

In case you still don't want g_set_application you can alternatively
bump the GTK+ requirement to 2.12 and use the "program-name" property

> You can still fill in the code for the URL and email hooks to make the
> actually work. I didn't bother so far, because the default screen thing
> seem to be wrong. You should use the screen where the actual about
> dialog is present. However I am not so deep into GDK to tell what is the
> correct way here.
>
I'm no expert either on that. I copied the code from network manager.
Leaving it as it is now in CVS (i.e. about dialog shows the links but
they don't work) doesn't seem like a good idea to me because users are
likely to run into this issue. I'd recommend one of the two following
1. Remove gtk_about_dialog_set_url_hook and gtk_about_dialog_set_email_hook
2. Use the code of the patch I sent you. It might be correct because
it's in Network Manager Applet.

Jaap

[1] http://bugzilla.gnome.org/show_bug.cgi?id=345822
[2] http://library.gnome.org/devel/glib/unstable/glib-Miscellaneous-Utility-Functions.html#g-set-application-name

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

Index: applet/main.c
===================================================================
RCS file: /cvsroot/bluez/gnome/applet/main.c,v
retrieving revision 1.94
diff -u -r1.94 main.c
--- applet/main.c	19 Dec 2007 01:06:05 -0000	1.94
+++ applet/main.c	20 Dec 2007 21:38:11 -0000
@@ -1524,48 +1524,23 @@
 		"Bastien Nocera <hadess@hadess.net>",
 		NULL
 	};
-	GtkWidget *dialog;
-
-	dialog = gtk_about_dialog_new();
-
-	gtk_window_set_icon_name(GTK_WINDOW(dialog), "stock_bluetooth");
-
-	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-2007 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),
-							"stock_bluetooth");
+	const char *translators;
+	translators = _("translator-credits");
 
 	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_show_about_dialog(NULL,
+			      "version", VERSION,
+			      "copyright", "Copyright \xc2\xa9 2005-2007 Marcel Holtmann",
+			      "comments", _("A Bluetooth manager for the GNOME desktop"),
+			      "authors", authors,
+			      "translator-credits", translators,
+			      "website", "http://www.bluez.org",
+			      "website-label", _("Bluez Website"),
+			      "logo-icon-name", "stock_bluetooth",
+			      NULL);
 }
 
 static void settings_callback(GObject *widget, gpointer user_data)
@@ -1834,6 +1809,8 @@
 	if (instance == NULL)
 		gtk_exit(0);
 
+	g_set_application_name (_("Bluetooth Applet"));
+
 	gtk_window_set_default_icon_name("stock_bluetooth");
 
 #ifdef HAVE_LIBNOTIFY

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

Index: applet/main.c
===================================================================
RCS file: /cvsroot/bluez/gnome/applet/main.c,v
retrieving revision 1.94
diff -u -r1.94 main.c
--- applet/main.c	19 Dec 2007 01:06:05 -0000	1.94
+++ applet/main.c	20 Dec 2007 21:34:20 -0000
@@ -1524,48 +1524,44 @@
 		"Bastien Nocera <hadess@hadess.net>",
 		NULL
 	};
-	GtkWidget *dialog;
-
-	dialog = gtk_about_dialog_new();
-
-	gtk_window_set_icon_name(GTK_WINDOW(dialog), "stock_bluetooth");
-
-	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-2007 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),
-							"stock_bluetooth");
+	const char *translators;
+	translators = _("translator-credits");
+ 
+	const char *license[] = {
+		N_("This program is free software; you can redistribute it and/or modify "
+		"it under the terms of the GNU General Public License as published by "
+		"the Free Software Foundation; either version 2 of the License, or "
+		"(at your option) any later version.\n"),
+		N_("This program is distributed in the hope that it will be useful, "
+		"but WITHOUT ANY WARRANTY; without even the implied warranty of "
+		"MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the "
+		"GNU General Public License for more details.\n"),
+		N_("You should have received a copy of the GNU General Public License "
+		"along with this program. If not, see <http://www.gnu.org/licenses/>.")
+	};
+
+	char *license_trans;
+        
+	license_trans = g_strconcat(_(license[0]), "\n", _(license[1]), "\n",
+				    _(license[2]), "\n", 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"));
-
-	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_show_about_dialog(NULL,
+			      "version", VERSION,
+			      "copyright", "Copyright \xc2\xa9 2005-2007 Marcel Holtmann",
+			      "comments", _("A Bluetooth manager for the GNOME desktop"),
+			      "authors", authors,
+			      "translator-credits", translators,
+			      "website", "http://www.bluez.org",
+			      "website-label", _("Bluez Website"),
+			      "logo-icon-name", "stock_bluetooth",
+			      "wrap-license", TRUE,
+			      "license", license_trans,
+			      NULL);
+	g_free(license_trans);
 }
 
 static void settings_callback(GObject *widget, gpointer user_data)
@@ -1834,6 +1830,8 @@
 	if (instance == NULL)
 		gtk_exit(0);
 
+	g_set_application_name (_("Bluetooth Applet"));
+
 	gtk_window_set_default_icon_name("stock_bluetooth");
 
 #ifdef HAVE_LIBNOTIFY

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

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

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

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