Re: Online help
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 <cjwatson@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 <jgg@debian.org>
(derived portions are public domain)
-CDebConf is copyrighted (c) 2000-2007 by Randolph Chung <tausq@debian.org>
-and the d-i team (see above) under the following license:
+CDebConf is copyrighted (c) 2000-2009 by Randolph Chung <tausq@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 [cjwatson@debian.org]
Reply to: