Re: [bug-sourceinstall] Autochecking the options in package configuration

[Claudio Fontana]

On Tue, Jun 24, 2008 at 4:00 PM, Nicola Fontana <address@hidden> wrote:
> On Tue, 24 Jun 2008 00:20:56 +0200
> "Claudio Fontana" <address@hidden> wrote:
>
>> >> My idea is to connect a signal to the keypress event of the
>> >> GtkEntry and to check if the value is != "\0", in which case the
>> >> checkbox will be set.
>>
>> Btw, the "changed" signal of the GtkEditable interface seems
>> a good place to start - Cld
>
> More than "a good place" it is "the place"...
>
> After my quick overview I was thinking GtkTreeView and
> renderers were used, but I found plain GtkEntry and
> GtkCheckButton.
> This means the patch was quite easy to do: I
> spent more time in building a proper patch file than modifying
> the code.
>
> Let me know if it works properly...

Comments inline:

> diff -x CVS -aur sourceinstall2-cvs/sourceinstall-gtk/dialog.c 
> sourceinstall2-ntd/sourceinstall-gtk/dialog.c
> --- sourceinstall2-cvs/sourceinstall-gtk/dialog.c     2008-06-23 
> 20:52:07.000000000 +0200
> +++ sourceinstall2-ntd/sourceinstall-gtk/dialog.c     2008-06-24 
> 15:42:57.000000000 +0200
> @@ -478,6 +478,18 @@
>      return rv;
>  }
>
> +/* Autocheck feature:
> +   activates the check button of an option if its value is not empty. */
> +static void autocheck(GtkWidget *entry, GtkWidget *check_button)
> +{
> +    const gchar *text;
> +
> +    text = gtk_entry_get_text(entry);
> +
> +    if (text[0] != '\0')
> +     gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (check_button), TRUE);
> +}
> +

style: gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(check_button), TRUE)
(without the spaces).

>  /* show configuration dialog, return choices in the passed string list.
>     Return 0 if the user cancelled the operation.
>     Writes the chosen configuration strings to S, and if non-zero
> @@ -491,7 +503,7 @@
>      GtkTooltips *tips;
>      gint response;
>      int i, j;
> -    const char *title = _("Package Configuration");
> +    char *title = _("Package Configuration");

why? maybe your CVS working copy is old.

>      struct {
>       char *title;
> @@ -562,6 +574,7 @@
>
>       for (j = 0; j < columns[i].set->count; j++) {
>           GtkWidget *horizontal, *label;
> +         GtkWidget *entry, *check_button;
>           char *text;
>           char *key, *param, *comment;
>
> @@ -569,20 +582,22 @@
>           gtk_box_pack_start(GTK_BOX(vertical), horizontal, FALSE, FALSE,
>                              0);
>
> -         columns[i].check_buttons[j] = gtk_check_button_new();
> -         columns[i].entries[j] = gtk_entry_new();
> -         gtk_box_pack_end(GTK_BOX(horizontal), columns[i].entries[j],
> -                          FALSE, FALSE, 0);
> +         check_button = gtk_check_button_new();
> +         entry = gtk_entry_new();
> +
> +         gtk_box_pack_end(GTK_BOX(horizontal), entry, FALSE, FALSE, 0);
>
> -         gtk_entry_set_width_chars(GTK_ENTRY(columns[i].entries[j]),
> +         gtk_entry_set_width_chars(GTK_ENTRY(entry),
>                                     i == 5 ? 20 : i == 2 ? 15 : 10);
> +         g_signal_connect_object (entry, "changed", G_CALLBACK (autocheck),
> +                                  check_button, 0);
>
>           gtk_box_pack_start(GTK_BOX(horizontal),
> -                            columns[i].check_buttons[j], FALSE, FALSE,
> +                            check_button, FALSE, FALSE,
>                              0);
>
>           label = gtk_label_new(columns[i].set->options[j].key);
> -         gtk_container_add(GTK_CONTAINER(columns[i].check_buttons[j]),
> +         gtk_container_add(GTK_CONTAINER(check_button),
>                             label);
>
>           key = columns[i].set->options[j].key;
> @@ -591,8 +606,7 @@
>
>           if (i == 5 && j == 0 && state.o.prefix) {
>               /* pre set prefix option value */
> -             gtk_entry_set_text(GTK_ENTRY(columns[i].entries[j]),
> -                                state.o.prefix);
> +             gtk_entry_set_text(GTK_ENTRY(entry), state.o.prefix);
>
>           } else if (old_configured) {
>               /* an option string (non-prefix) must always be preceded
> @@ -607,15 +621,14 @@
>                   (start[len] == ' ' || start[len] == '=')) {
>
>                   gtk_toggle_button_set_active(
> -                      GTK_TOGGLE_BUTTON(columns[i].check_buttons[j]), TRUE);
> +                      GTK_TOGGLE_BUTTON(check_button), TRUE);
>
>                   if (start[len] == '=') {
>                       char *end;
>                       start = srcinst_strdup(start + len + 1);
>                       if ((end = strchr(start, ' '))) {
>                           *end = 0;
> -                         gtk_entry_set_text(GTK_ENTRY(columns[i].entries[j]),
> -                                            start);
> +                         gtk_entry_set_text(GTK_ENTRY(entry), start);
>                       }
>                       free(start);
>                   }
> @@ -627,10 +640,12 @@
>                               param ? param : "", param ? "]\n" : "\n",
>                               comment ? comment : "", NULL);
>
> -         gtk_tooltips_set_tip(GTK_TOOLTIPS(tips),
> -                              columns[i].check_buttons[j], text, 0);
> +         gtk_tooltips_set_tip(GTK_TOOLTIPS(tips), check_button, text, 0);
>
>           free(text);
> +
> +         columns[i].check_buttons[j] = check_button;
> +         columns[i].entries[j] = entry;
>       }
>      }
>

This seems all ok. After those two small cleanups,
please provide the full patch again and a separate ChangeLog
entry with your name, to properly credit your contribution.
The patch must not contain the ChangeLog changes.

After the name and mail address, add the string '(tiny change)' as to
avoid having to sign assignment papers for the changes.

Then I can commit that. Sorry for the bureaucracy, but it is necessary
for GNU projects.

Ciao,

Claudio