On Wednesday 12 of December 2012 17:07:33 Denys Vlasenko wrote:
> On 12/12/2012 11:27 AM, Jakub Filak wrote:
> > - related to trac#916
> >
> > Signed-off-by: Jakub Filak <jfilak@redhat.com>
>
> Is it a patch against some branch?
> I don't see src/gtk-helpers/ask_dialogs.c in main branch...
>
The previous patch added the file ... not pushed yet
>
> > diff --git a/src/gtk-helpers/ask_dialogs.c b/src/gtk-helpers/ask_dialogs.c
> > index 7994e1b..254ba07 100644
> > --- a/src/gtk-helpers/ask_dialogs.c
> > +++ b/src/gtk-helpers/ask_dialogs.c
> > @@ -33,18 +33,38 @@ static void on_toggle_ask_yes_no_yesforever_cb(GtkToggleButton *tb, gpointer use
> > gtk_widget_set_sensitive(GTK_WIDGET(user_data), !gtk_toggle_button_get_active(tb));
> > }
> >
> > -/*
> > - * Function shows a dialog with 'Yes/No' buttons and a check box allowing to
> > - * remember the answer. The "Don't ask me again" response is stored in
> > - * configuration file under 'key'.
> > - */
> > -int run_ask_yes_no_yesforever_dialog(const char *key, const char *message, GtkWindow *parent)
> > +typedef enum {
> > + ASK_YES_NO_SAVE_RESULT = 1 << 0,
>
> I'd separate prefix from the remainder.
> Maybe with two underscores? - "ASK_YES_NO__SAVE_RESULT"?
>
will do
> > +
> > + /*
> > + * ASK_YES_NO_YESFOREVER means that you can reply with "Yes", "No" an "Yes
> > + * && Don't ask me again".
> > + *
> > + * If you check the checkbox the "No" button will be disabled and you will
> > + * be able to click only the "Yes" button.
> > + *
> > + * Once you answer "Yes && Don't ask me again", the dialog won't appear
> > + * next time you call the function and Non 0 value is returned immediately
> > + */
> > + ASK_YES_NO_YESFOREVER = 1 << 1,
>
> I propose to call it ASK_YES_NO__DISALLOW_SAVING_NEGATIVE
> (naturally, it requires ASK_YES_NO__SAVE_RESULT bit to be meaningful).
>
> > +} ask_yes_no_dialog_flags;
> > +
> > +static int run_ask_yes_no_save_generic_result_dialog(ask_yes_no_dialog_flags flags,
> > + const char *key,
> > + const char *message,
> > + GtkWindow *parent)
> > {
> > const char *ask_result = get_user_setting(key);
> >
> > - if (ask_result && string_to_bool(ask_result) == false)
> > - /* Do you want to be asked? -> No, I don't. Do whatever you want */
> > - return true;
> > + if (ask_result)
> > + {
> > + const bool ret = string_to_bool(ask_result);
> > + if ((flags & ASK_YES_NO_YESFOREVER) && ret == false)
> > + /* Do you want to be asked? -> No, I don't. Do whatever you want */
> > + return true;
> > +
> > + return ret;
> > + }
>
> How about making the above part simpler, like this:
>
> const char *ask_result = get_user_setting(key);
> if (ask_result)
> return string_to_bool(ask_result);
>
> In other words, user setting does not remember "Do you want to be asked?"
> bit - it remembers the actual answer ("yes" or "no").
> If user setting _does not exist_, then, naturally, it means
> "we want to be asked".
>
> The rest of the code will become somewhat simpler as a result,
> since now the meaning of user setting does not depend on
> whether you DISALLOW_SAVING_NEGATIVE or not.
I'd really love to do it in that way but libreport has been using this version
for some time and this change would break the current user configuration.
>
>