On Thu, Jun 25, 2009 at 12:09:44PM +0100, Colin Watson wrote: > Here's an initial draft of a patch for this, tested interactively using > the changes to src/test/help.templates included in the patch. It was > surprisingly easy! I've only done the newt interface so far, not the GTK > interface; please have a look over this, since I'd like to have general > consensus that I'm on the right track before going any further.
Here's a second version with added support for the text and GTK frontends, and a fix for a reference leak in the newt frontend implementation. Please review. I'd like to commit this towards the end of this week if there are no objections. Index: debian/changelog =================================================================== --- debian/changelog (revision 59150) +++ debian/changelog (working copy) @@ -1,3 +1,12 @@ +cdebconf (0.143) UNRELEASED; urgency=low + + * Add online help support. The "Help" field in a template may refer to + another template whose description contains help text; frontends may use + this to display online help on request. Currently implemented for the + text, newt, and gtk frontends. + + -- Colin Watson <cjwat...@debian.org> Mon, 29 Jun 2009 13:04:55 +0100 + cdebconf (0.142) unstable; urgency=low [ Frans Pop ] Index: debian/cdebconf-newt-udeb.templates =================================================================== --- debian/cdebconf-newt-udeb.templates (revision 59150) +++ debian/cdebconf-newt-udeb.templates (working copy) @@ -33,4 +33,11 @@ # Help line displayed at the bottom of the cdebconf newt interface. # Translators: must fit within 80 characters. # :sl1: -_Description: <Tab> moves between items; <Space> selects; <Enter> activates buttons +_Description: <Tab> moves; <Space> selects; <Enter> activates buttons + +Template: debconf/help-line-f1 +Type: text +# Help line displayed at the bottom of the cdebconf newt interface. +# Translators: must fit within 80 characters. +# :sl1: +_Description: <F1> for help; <Tab> moves; <Space> selects; <Enter> activates buttons Index: debian/cdebconf-gtk-udeb.templates =================================================================== --- debian/cdebconf-gtk-udeb.templates (revision 59150) +++ debian/cdebconf-gtk-udeb.templates (working copy) @@ -22,6 +22,12 @@ # :sl1: _Description: No +Template: debconf/button-help +Type: text +# Translators, this text will appear on a button, so KEEP IT SHORT +# :sl1: +_Description: Help + Template: debconf/text-direction Type: text # TRANSLATORS: This should be "translated" to "RTL" or "LTR" depending of Index: debian/copyright =================================================================== --- debian/copyright (revision 59150) +++ debian/copyright (working copy) @@ -15,8 +15,8 @@ (c) Jason Gunthorpe <j...@debian.org> (derived portions are public domain) -CDebConf is copyrighted (c) 2000-2007 by Randolph Chung <ta...@debian.org> -and the d-i team (see above) under the following license: +CDebConf is copyrighted (c) 2000-2009 by Randolph Chung <ta...@debian.org>, +the d-i team (see above), and Canonical Ltd. under the following license: Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions Index: src/template.c =================================================================== --- src/template.c (revision 59150) +++ src/template.c (working copy) @@ -21,6 +21,7 @@ "indices", "description", "extended_description", + "help", NULL }; @@ -134,6 +135,7 @@ DELETE(t->tag); DELETE(t->type); + DELETE(t->help); p = t->fields; DELETE(t); while (p != NULL) @@ -175,6 +177,7 @@ struct template_l10n_fields *from, *to; ret->type = STRDUP(t->type); + ret->help = STRDUP(t->help); if (t->fields == NULL) return ret; @@ -349,7 +352,7 @@ * Input: a field name * Output: the value of the given field in the given language, field * name may be any of type, default, choices, indices, - * description and extended_description + * description, extended_description and help * Description: get field value * Assumptions: */ @@ -367,6 +370,8 @@ return t->tag; else if (strcasecmp(field, "type") == 0) return t->type; + else if (strcasecmp(field, "help") == 0) + return t->help; /* If field is Foo-xx.UTF-8 then call template_lget(t, "xx", "Foo") */ if (strchr(field, '-') != NULL) @@ -428,7 +433,7 @@ * Input: a field name * Output: the value of the given field in the given language, field * name may be any of type, default, choices, indices, - * description and extended_description + * description, extended_description and help * Description: get field value * Assumptions: Arguments have been previously checked, lang and field * are not NULL @@ -478,6 +483,11 @@ t->type = STRDUP(value); return; } + else if (strcasecmp(field, "help") == 0) + { + t->help = STRDUP(value); + return; + } /* If field is Foo-xx.UTF-8 then call template_lset(t, "xx", "Foo") */ if (strchr(field, '-') != NULL) @@ -676,6 +686,8 @@ t = template_new(p+10); else if (strstr(p, "Type: ") == p && t != 0) template_lset(t, NULL, "type", p+6); + else if (strstr(p, "Help: ") == p && t != 0) + template_lset(t, NULL, "help", p+6); else if (strstr(p, "Default: ") == p && t != 0) template_lset(t, NULL, "default", p+9); else if (i18n && strstr(p, "Default-") == p && t != 0) Index: src/template.h =================================================================== --- src/template.h (revision 59150) +++ src/template.h (working copy) @@ -23,6 +23,7 @@ char *tag; unsigned int ref; char *type; + char *help; struct template_l10n_fields *fields; struct template *next; }; Index: src/modules/frontend/text/text.c =================================================================== --- src/modules/frontend/text/text.c (revision 59150) +++ src/modules/frontend/text/text.c (working copy) @@ -171,6 +171,24 @@ show_help (struct frontend *obj, struct question *q) { char *descr = q_get_description(obj, q); + char *help = q_get_help(obj, q); + if (*help) { + struct question *help_q = obj->qdb->methods.get(obj->qdb, help); + if (help_q) { + char *help_descr = q_get_description(obj, help_q); + char *help_ext_descr = q_get_extended_description(obj, help_q); + wrap_print(help_descr); + printf("\n"); + if (*help_ext_descr) { + wrap_print(help_ext_descr); + printf("\n"); + } + free(help_ext_descr); + free(help_descr); + question_deref(help_q); + } + free(help); + } printf("%s", question_get_text(obj, "debconf/text-help-keystrokes", "KEYSTROKES:")); printf("\n %c ", CHAR_HELP); printf("%s", question_get_text(obj, "debconf/text-help-help", "Display this help message")); Index: src/modules/frontend/gtk/linker-script =================================================================== --- src/modules/frontend/gtk/linker-script (revision 59150) +++ src/modules/frontend/gtk/linker-script (working copy) @@ -18,6 +18,7 @@ cdebconf_gtk_add_buttons; cdebconf_gtk_set_button_secondary; cdebconf_gtk_create_continue_button; + cdebconf_gtk_help; local: *; }; Index: src/modules/frontend/gtk/go.c =================================================================== --- src/modules/frontend/gtk/go.c (revision 59150) +++ src/modules/frontend/gtk/go.c (working copy) @@ -139,6 +139,48 @@ fe_data->setters = NULL; } +/** Key event handler implementing the "Help" key shortcut. + * + * @param widget main widget + * @param key pressed key + * @param fe cdebconf frontend + * @return TRUE if "Help" shortcut has been pressed, FALSE otherwise + */ +static gboolean handle_help_key(GtkWidget * widget, GdkEventKey * key, + struct frontend * fe) +{ + if (GDK_F1 == key->keyval || GDK_KP_F1 == key->keyval) { + cdebconf_gtk_help(fe); + return TRUE; + } + return FALSE; +} + +/** Create the "Help" button. + * + * The button will be added to the main action box. + * + * @param fe cdebconf frontend + */ +static void create_help_button(struct frontend * fe) +{ + GtkWidget * button; + char * label; + + label = cdebconf_gtk_get_text(fe, "debconf/button-help", "Help"); + /* XXX: check NULL! */ + button = gtk_button_new_with_label(label); + g_free(label); + + g_signal_connect_swapped(G_OBJECT(button), "clicked", + G_CALLBACK(cdebconf_gtk_help), fe); + + cdebconf_gtk_add_button(fe, button); + cdebconf_gtk_set_button_secondary(fe, button, TRUE); + cdebconf_gtk_add_global_key_handler( + fe, button, G_CALLBACK(handle_help_key)); +} + /** Key event handler implementing the "Go Back" key shortcut. * * @param widget main window @@ -488,6 +530,7 @@ { struct frontend_data * fe_data = fe->data; GtkWidget * question_box; + struct question * question; int ret; if (NULL == fe->questions) { @@ -495,6 +538,7 @@ } cdebconf_gtk_set_answer(fe, DC_NO_ANSWER); + fe_data->help_question = NULL; gdk_threads_enter(); #ifdef DI_UDEB @@ -519,6 +563,25 @@ if (!is_action_box_filled(fe)) { create_default_buttons(fe); } + + /* Set up the help button if necessary. FIXME: We only offer help for + * the first question with help text in any given question box. It's not + * clear what the right thing to do here is. + */ + question = fe->questions; + while (NULL != question) { + char *help = q_get_help(fe, question); + if (help) { + struct question *help_q = fe->qdb->methods.get(fe->qdb, help); + if (help_q) { + fe_data->help_question = help_q; + create_help_button(fe); + break; + } + } + question = question->next; + } + cdebconf_gtk_show_target_box(fe); cdebconf_gtk_show_buttons(fe); gdk_threads_leave(); @@ -544,6 +607,8 @@ gdk_threads_leave(); end: + question_deref(fe_data->help_question); + fe_data->help_question = NULL; free_setters(fe_data); return fe_data->answer; } Index: src/modules/frontend/gtk/cdebconf_gtk.c =================================================================== --- src/modules/frontend/gtk/cdebconf_gtk.c (revision 59150) +++ src/modules/frontend/gtk/cdebconf_gtk.c (working copy) @@ -301,6 +301,23 @@ cdebconf_gtk_set_answer(fe, DC_GOBACK); } +/* documented in cdebconf_gtk.h */ +void cdebconf_gtk_help(struct frontend * fe) +{ + struct frontend_data * fe_data = fe->data; + char * description; + char * ext_description; + + if (NULL == fe_data || NULL == fe_data->help_question) + return; + + description = q_get_description(fe, fe_data->help_question); + ext_description = q_get_extended_description(fe, fe_data->help_question); + cdebconf_gtk_run_message_dialog(fe, description, ext_description); + g_free(ext_description); + g_free(description); +} + /** Create the event listener thread. * * @param fe cdebconf frontend Index: src/modules/frontend/gtk/cdebconf_gtk.h =================================================================== --- src/modules/frontend/gtk/cdebconf_gtk.h (revision 59150) +++ src/modules/frontend/gtk/cdebconf_gtk.h (working copy) @@ -161,6 +161,12 @@ */ void cdebconf_gtk_set_answer_goback(struct frontend * fe); +/** Display help for the current question. + * + * @param fe cdebconf frontend + */ +void cdebconf_gtk_help(struct frontend * fe); + /** Force cdebconf to quit. * * This function is currently used when the main window is closed. Index: src/modules/frontend/gtk/fe_data.h =================================================================== --- src/modules/frontend/gtk/fe_data.h (revision 59150) +++ src/modules/frontend/gtk/fe_data.h (working copy) @@ -118,6 +118,9 @@ */ GHashTable * plugins; + /** Current question for which help will be displayed. */ + struct question * help_question; + #ifdef DI_UDEB /** Internal data for specific handling related to the debian-installer. * Index: src/modules/frontend/newt/cdebconf_newt.h =================================================================== --- src/modules/frontend/newt/cdebconf_newt.h (revision 59150) +++ src/modules/frontend/newt/cdebconf_newt.h (working copy) @@ -16,7 +16,7 @@ */ char *cdebconf_newt_get_progress_info(struct frontend *obj); -#define cdebconf_newt_create_form(scrollbar) newtForm((scrollbar), NULL, 0) +newtComponent cdebconf_newt_create_form(newtComponent scrollbar); void cdebconf_newt_create_window(const int width, const int height, const char *title, const char *priority); Index: src/modules/frontend/newt/newt.c =================================================================== --- src/modules/frontend/newt/newt.c (revision 59150) +++ src/modules/frontend/newt/newt.c (working copy) @@ -117,6 +117,11 @@ typedef int (newt_handler)(struct frontend *obj, struct question *q); +struct newt_help_callback_data { + struct frontend *obj; + const char *tag; +}; + static void newt_progress_stop(struct frontend *obj); /* Result must be freed by the caller */ @@ -171,9 +176,15 @@ static const char * help_text(struct frontend *obj) { - return question_get_text(obj, "debconf/help-line", "<Tab> moves between items; <Space> selects; <Enter> activates buttons"); + return question_get_text(obj, "debconf/help-line", "<Tab> moves; <Space> selects; <Enter> activates buttons"); } +static const char * +help_text_f1(struct frontend *obj) +{ + return question_get_text(obj, "debconf/help-line", "<F1> for help; <Tab> moves; <Space> selects; <Enter> activates buttons"); +} + void cdebconf_newt_setup(void) { @@ -196,6 +207,17 @@ return NULL; } +/* This is global due to cdebconf_newt_create_form's interface. It might be + * worth fixing this at the next convenient API break. + */ +static struct newt_help_callback_data *help_cb_data; + +newtComponent +cdebconf_newt_create_form(newtComponent scrollbar) +{ + return newtForm(scrollbar, help_cb_data, 0); +} + void cdebconf_newt_create_window(const int width, const int height, const char *title, const char *priority) { @@ -346,7 +368,7 @@ } static int -show_separate_window(struct frontend *obj, struct question *q) +show_separate_window(struct frontend *obj, struct question *q, int is_help) { newtComponent form, textbox, bOk, bCancel, cRet; int width = 80, height = 24, t_height, t_width, win_width, win_height; @@ -403,7 +425,7 @@ if (t_width_descr > t_width) t_width = t_width_descr; t_width_buttons = 2*BUTTON_PADDING + cdebconf_newt_get_text_width(continue_text(obj)) + 2; - if (obj->methods.can_go_back(obj, q)) + if (!is_help && obj->methods.can_go_back(obj, q)) // Add an interspace t_width_buttons += cdebconf_newt_get_text_width(goback_text(obj)) + 3; if (t_width_buttons > t_width) @@ -421,7 +443,7 @@ assert(textbox); newtTextboxSetText(textbox, full_description); free(full_description); - if (obj->methods.can_go_back(obj, q)) { + if (!is_help && obj->methods.can_go_back(obj, q)) { bOk = newtCompactButton(win_width - TEXT_PADDING - BUTTON_PADDING - strwidth(continue_text(obj)) - 3, win_height-2, continue_text(obj)); bCancel = newtCompactButton(TEXT_PADDING + BUTTON_PADDING - 1, win_height-2, goback_text(obj)); newtFormAddComponents(form, bCancel, textbox, bOk, NULL); @@ -952,7 +974,7 @@ separate_window = need_separate_window(obj, q); while (1) { if (separate_window) { - ret = show_separate_window(obj, q); + ret = show_separate_window(obj, q, 0); if (ret != DC_OK) return ret; ret = show_multiselect_window(obj, q, 0); @@ -980,7 +1002,7 @@ separate_window = need_separate_window(obj, q); while (1) { if (separate_window) { - ret = show_separate_window(obj, q); + ret = show_separate_window(obj, q, 0); if (ret != DC_OK) return ret; ret = show_select_window(obj, q, 0); @@ -1030,7 +1052,7 @@ static int newt_handler_note(struct frontend *obj, struct question *q) { // Sort of hack-ish, yes, but it works :-) - return show_separate_window(obj, q); + return show_separate_window(obj, q, 0); } /* @@ -1074,6 +1096,23 @@ return ret; } +static void +newt_help_callback(newtComponent co, void *d) +{ + struct newt_help_callback_data *data = d; + struct frontend *obj = data->obj; + struct question *q = obj->qdb->methods.get(obj->qdb, data->tag); + + /* Don't allow recursive calls. */ + newtSetHelpCallback(NULL); + + show_separate_window(obj, q, 1); + + newtSetHelpCallback(newt_help_callback); + + question_deref(q); +} + /* ----------------------------------------------------------------------- */ struct question_handlers { const char *type; @@ -1158,21 +1197,37 @@ } if (plugin || strcmp(q->template->type, question_handlers[i].type) == 0) { + char *help = q_get_help(obj, q); + struct newt_help_callback_data my_help_cb_data; if (!cleared && !data->scale_form) { cleared = 1; cdebconf_newt_setup(); } + if (*help) { + my_help_cb_data.obj = obj; + my_help_cb_data.tag = help; + help_cb_data = &my_help_cb_data; + newtSetHelpCallback(newt_help_callback); + } if (obj->info != NULL) { char *text = q_get_description(obj, obj->info); if (text) newtDrawRootText(0, 0, text); free(text); } - newtPushHelpLine(help_text(obj)); + if (*help) + newtPushHelpLine(help_text_f1(obj)); + else + newtPushHelpLine(help_text(obj)); ret = handler(obj, q); newtPopHelpLine(); if (ret == DC_OK) obj->qdb->methods.set(obj->qdb, q); + if (*help) { + newtSetHelpCallback(NULL); + help_cb_data = NULL; + } + free(help); if (plugin) plugin_delete(plugin); break; Index: src/modules/db/mysql/create.sql =================================================================== --- src/modules/db/mysql/create.sql (revision 59150) +++ src/modules/db/mysql/create.sql (working copy) @@ -33,6 +33,7 @@ choices CHAR(255), description CHAR(80), ext_description CHAR(255), + help CHAR(80), modified TIMESTAMP, PRIMARY KEY(id), KEY idx_Templates_tag(tag) Index: src/test/help.templates =================================================================== --- src/test/help.templates (revision 59150) +++ src/test/help.templates (working copy) @@ -1,8 +1,14 @@ +Template: test-help/help +Type: text +Description: Help text + This is some help text. + Template: test-help/boolean Type: boolean Default: true +Help: test-help/help Description: Enter a boolean value - This is a boolean question. + This is a boolean question with some help text. Template: test-help/error Type: error Index: src/question.h =================================================================== --- src/question.h (revision 59150) +++ src/question.h (working copy) @@ -13,6 +13,7 @@ #define q_get_choices(fe, q) question_get_field((fe), (q), "", "choices") #define q_get_choices_vals(fe, q) question_get_raw_field((q), "C", "choices") #define q_get_indices(fe, q) question_get_field((fe), (q), "", "indices") +#define q_get_help(fe, q) question_get_raw_field((q), "", "help") #define q_get_raw_extended_description(q) question_get_raw_field((q), "", "extended_description") #define q_get_raw_description(q) question_get_raw_field((q), "", "description") #define q_get_raw_choices(q) question_get_raw_field((q), "", "choices") -- Colin Watson [cjwat...@debian.org] -- To UNSUBSCRIBE, email to debian-boot-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org