Hi Vincent, I wrote most of gnome-session and am still listed as a maintainer. So, I'm not sure what the problem is. I agree in principle with using code review which is why I asked a few people to look the patch over for me.
This is crunch time man. It is all about getting it done. Thanks, Jon On Mon, Feb 28, 2011 at 7:56 AM, Vincent Untz <[email protected]> wrote: > Hi, > > (I don't want to pick on Jon here; that's just the latest commit like > this one) > > I would appreciate if patches could get posted on bugzilla for review, > instead of being committed directly without asking maintainers. I know > I'm not the fastest reviewer out there, but as I've told several times > before, if you think the patch is important and doesn't get a review > before the next release, then it's welcome to push the patch. > > In this case, the patch is mostly good, but the man page is wrong > (--logout is the default behavior, but it's not mentioned and it's > unclear what happens by default; --no-prompt doesn't do anything with > --power-off). > > Vincent > > Le jeudi 24 février 2011, à 22:56 +0000, William Jon McCann a écrit : >> commit 8663860ff44b9a5e441e4909a49eee4cfa08378d >> Author: William Jon McCann <[email protected]> >> Date: Thu Feb 24 17:38:13 2011 -0500 >> >> rename gnome-session-save to gnome-session-quit >> >> Is much less misleading since it doesn't save anything. >> >> doc/man/Makefile.am | 2 +- >> doc/man/gnome-session-quit.1 | 25 ++++ >> doc/man/gnome-session-save.1 | 40 ------- >> po/POTFILES.in | 2 +- >> tools/Makefile.am | 10 +- >> .../{gnome-session-save.c => gnome-session-quit.c} | 116 >> +++---------------- >> 6 files changed, 51 insertions(+), 144 deletions(-) >> --- >> diff --git a/doc/man/Makefile.am b/doc/man/Makefile.am >> index 72f9a93..e42430a 100644 >> --- a/doc/man/Makefile.am >> +++ b/doc/man/Makefile.am >> @@ -1,7 +1,7 @@ >> man_MANS = \ >> gnome-session.1 \ >> gnome-session-properties.1 \ >> - gnome-session-save.1 >> + gnome-session-quit.1 >> >> EXTRA_DIST = \ >> $(man_MANS) >> diff --git a/doc/man/gnome-session-quit.1 b/doc/man/gnome-session-quit.1 >> new file mode 100644 >> index 0000000..2f6df84 >> --- /dev/null >> +++ b/doc/man/gnome-session-quit.1 >> @@ -0,0 +1,25 @@ >> +.\" >> +.\" gnome-session-quit manual page. >> +.\" (C) 2000 Miguel de Icaza ([email protected]) >> +.\" (C) 2009-2010 Vincent Untz ([email protected]) >> +.\" >> +.TH GNOME-SESSION-QUIT 1 "GNOME" >> +.SH NAME >> +gnome-session-quit \- End the current GNOME session >> +.SH SYNOPSIS >> +.B gnome-session-quit [\-\-logout] [\-\-power-off] [\-\-no-prompt] >> +.SH DESCRIPTION >> +The \fIgnome-session-quit\fP program can be used to end a GNOME session. >> +.PP >> +If called with the \fB\-\-logout\fP option the user will be prompted >> +to confirm logout. The \fB\-\-no\-prompt\fP option can be used to end >> +the session without user interaction. >> +.PP >> +When the \fB\-\-power\-off\fP option is given the user will be >> +prompted to confirm system power off. The \fB\-\-no\-prompt\fP option >> +can be used to power off without user interaction. >> +.SH BUGS >> +If you find bugs in the \fIgnome-session-quit\fP program, please report >> +these on https://bugzilla.gnome.org. >> +.SH SEE ALSO >> +.BR gnome-session(1) >> diff --git a/po/POTFILES.in b/po/POTFILES.in >> index 4b3a1a1..1e7491d 100644 >> --- a/po/POTFILES.in >> +++ b/po/POTFILES.in >> @@ -20,4 +20,4 @@ gnome-session/gsm-xsmp-client.c >> gnome-session/gsm-xsmp-server.c >> gnome-session/gsm-util.c >> gnome-session/main.c >> -tools/gnome-session-save.c >> +tools/gnome-session-quit.c >> diff --git a/tools/Makefile.am b/tools/Makefile.am >> index fbc41b5..f53a012 100644 >> --- a/tools/Makefile.am >> +++ b/tools/Makefile.am >> @@ -1,14 +1,14 @@ >> -bin_PROGRAMS = gnome-session-save >> +bin_PROGRAMS = gnome-session-quit >> libexec_PROGRAMS = gnome-session-is-accelerated >> >> AM_CPPFLAGS = >> >> AM_CFLAGS = $(WARN_CFLAGS) >> >> -gnome_session_save_SOURCES = \ >> - gnome-session-save.c >> +gnome_session_quit_SOURCES = \ >> + gnome-session-quit.c >> >> -gnome_session_save_CPPFLAGS = \ >> +gnome_session_quit_CPPFLAGS = \ >> $(AM_CPPFLAGS) \ >> $(GNOME_SESSION_CFLAGS) \ >> $(DBUS_GLIB_CFLAGS) \ >> @@ -16,7 +16,7 @@ gnome_session_save_CPPFLAGS = \ >> -DLOCALE_DIR=\""$(datadir)/locale"\" \ >> $(DISABLE_DEPRECATED_CFLAGS) >> >> -gnome_session_save_LDADD = \ >> +gnome_session_quit_LDADD = \ >> $(SM_LIBS) \ >> $(ICE_LIBS) \ >> $(GNOME_SESSION_LIBS) \ >> diff --git a/tools/gnome-session-save.c b/tools/gnome-session-quit.c >> similarity index 59% >> rename from tools/gnome-session-save.c >> rename to tools/gnome-session-quit.c >> index 28f6443..f5d1838 100644 >> --- a/tools/gnome-session-save.c >> +++ b/tools/gnome-session-quit.c >> @@ -43,54 +43,23 @@ enum { >> GSM_LOGOUT_MODE_FORCE >> }; >> >> -/* True if killing. This is deprecated, but we keep it for compatibility >> - * reasons. */ >> -static gboolean kill_session = FALSE; >> - >> -/* The real options that should be used now. They are not ambiguous. */ >> static gboolean logout = FALSE; >> -static gboolean force_logout = FALSE; >> -static gboolean logout_dialog = FALSE; >> -static gboolean shutdown_dialog = FALSE; >> - >> -/* True if we should use dialog boxes */ >> -static gboolean show_error_dialogs = FALSE; >> - >> -/* True if we should do the requested action without confirmation */ >> -static gboolean no_interaction = FALSE; >> - >> -static char *session_name = NULL; >> +static gboolean power_off = FALSE; >> +static gboolean no_prompt = FALSE; >> +static gboolean force = FALSE; >> >> static GOptionEntry options[] = { >> {"logout", '\0', 0, G_OPTION_ARG_NONE, &logout, N_("Log out"), >> NULL}, >> - {"force-logout", '\0', 0, G_OPTION_ARG_NONE, &force_logout, N_("Log >> out, ignoring any existing inhibitors"), NULL}, >> - {"logout-dialog", '\0', 0, G_OPTION_ARG_NONE, &logout_dialog, >> N_("Show logout dialog"), NULL}, >> - {"shutdown-dialog", '\0', 0, G_OPTION_ARG_NONE, &shutdown_dialog, >> N_("Show shutdown dialog"), NULL}, >> - {"gui", '\0', 0, G_OPTION_ARG_NONE, &show_error_dialogs, N_("Use >> dialog boxes for errors"), NULL}, >> - /* deprecated options */ >> - {"session-name", 's', G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_STRING, >> &session_name, N_("Set the current session name"), N_("NAME")}, >> - {"kill", '\0', G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_NONE, >> &kill_session, N_("Kill session"), NULL}, >> - {"silent", '\0', G_OPTION_FLAG_HIDDEN, G_OPTION_ARG_NONE, >> &no_interaction, N_("Do not require confirmation"), NULL}, >> + {"power-off", '\0', 0, G_OPTION_ARG_NONE, &power_off, N_("Power >> off"), NULL}, >> + {"force", '\0', 0, G_OPTION_ARG_NONE, &force, N_("Ignoring any >> existing inhibitors"), NULL}, >> + {"no-prompt", '\0', 0, G_OPTION_ARG_NONE, &no_prompt, N_("Don't >> prompt for user confirmation"), NULL}, >> {NULL} >> }; >> >> static void >> display_error (const char *message) >> { >> - if (show_error_dialogs && !no_interaction) { >> - GtkWidget *dialog; >> - >> - dialog = gtk_message_dialog_new (NULL, 0, GTK_MESSAGE_ERROR, >> - GTK_BUTTONS_CLOSE, >> - "%s", message); >> - >> - /*gtk_window_set_default_icon_name (GTK_STOCK_SAVE);*/ >> - >> - gtk_dialog_run (GTK_DIALOG (dialog)); >> - gtk_widget_destroy (dialog); >> - } else { >> - g_printerr ("%s\n", message); >> - } >> + g_printerr ("%s\n", message); >> } >> >> static DBusGConnection * >> @@ -134,43 +103,6 @@ get_sm_proxy (void) >> return sm_proxy; >> } >> >> -#if 0 >> -static void >> -set_session_name (const char *session_name) >> -{ >> - DBusGProxy *sm_proxy; >> - GError *error; >> - gboolean res; >> - >> - sm_proxy = get_sm_proxy (); >> - if (sm_proxy == NULL) { >> - return; >> - } >> - >> - error = NULL; >> - res = dbus_g_proxy_call (sm_proxy, >> - "SetName", >> - &error, >> - G_TYPE_STRING, session_name, >> - G_TYPE_INVALID, G_TYPE_INVALID); >> - >> - if (!res) { >> - if (error != NULL) { >> - g_warning ("Failed to set session name '%s': %s", >> - session_name, error->message); >> - g_error_free (error); >> - } else { >> - g_warning ("Failed to set session name '%s'", >> - session_name); >> - } >> - } >> - >> - if (sm_proxy != NULL) { >> - g_object_unref (sm_proxy); >> - } >> -} >> -#endif >> - >> static void >> do_logout (unsigned int mode) >> { >> @@ -207,7 +139,7 @@ do_logout (unsigned int mode) >> } >> >> static void >> -do_shutdown_dialog (void) >> +do_power_off (void) >> { >> DBusGProxy *sm_proxy; >> GError *error; >> @@ -259,34 +191,24 @@ main (int argc, char *argv[]) >> } >> >> conflicting_options = 0; >> - if (kill_session) >> - conflicting_options++; >> if (logout) >> conflicting_options++; >> - if (force_logout) >> - conflicting_options++; >> - if (logout_dialog) >> - conflicting_options++; >> - if (shutdown_dialog) >> + if (power_off) >> conflicting_options++; >> if (conflicting_options > 1) >> display_error (_("Program called with conflicting >> options")); >> >> - if (kill_session) { >> - if (no_interaction) >> - force_logout = TRUE; >> - else >> - logout_dialog = TRUE; >> - } >> + if (power_off) { >> + do_power_off (); >> + } else { >> + /* default to logout */ >> >> - if (logout) { >> - do_logout (GSM_LOGOUT_MODE_NO_CONFIRMATION); >> - } else if (force_logout) { >> - do_logout (GSM_LOGOUT_MODE_FORCE); >> - } else if (logout_dialog) { >> - do_logout (GSM_LOGOUT_MODE_NORMAL); >> - } else if (shutdown_dialog) { >> - do_shutdown_dialog (); >> + if (force) >> + do_logout (GSM_LOGOUT_MODE_FORCE); >> + else if (no_prompt) >> + do_logout (GSM_LOGOUT_MODE_NO_CONFIRMATION); >> + else >> + do_logout (GSM_LOGOUT_MODE_NORMAL); >> } >> >> return 0; >> _______________________________________________ >> commits-list mailing list (read only) >> http://mail.gnome.org/mailman/listinfo/commits-list >> >> Want to limit the commits to a few modules? Go to above URL, log in to edit >> your options and select the modules ('topics') you want. > > -- > Les gens heureux ne sont pas pressés. > _______________________________________________ desktop-devel-list mailing list [email protected] http://mail.gnome.org/mailman/listinfo/desktop-devel-list
