[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Online help



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.

Adding a Help button in the newt frontend works out to be hideously more
work, as all the button handling is very much hard-coded and you'd have
to arrange for a way to loop back round to the question you were asking
after displaying the help window, rather than just letting newt do it
for you. On reflection, I prefer the press-F1 interface there anyway. By
contrast, in the GTK frontend, I think a Help button would be a better
interface. See e.g.
http://library.gnome.org/devel/hig-book/stable/windows-alert.html.en.

I have an untested patch to add the basic necessary support to debconf,
which at least ought to deal with passthrough so that we can use it in
things like tasksel. Adding actual UI to debconf's dialog frontend is
blocked on http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=534518.
Adding it to the GNOME frontend is blocked on somebody figuring out how
on earth to deal with it in the weird (and deprecated) druid interface
used there. I haven't looked at the KDE frontend. None of this is at all
high-priority for me.

I refrained from adding a capability indicating the availability of
online help, since I believe that applications should not generally be
relying on the presence of online help in designing their interface
(i.e. you shouldn't be saying "do I have online help? All right then, I
can make my interface really uninformative since they can always press
F1 to figure out what's going on" ...). If I'm mistaken here and there's
a legitimate use case for this, let me know and it should be easy to
add.

(This was done on Canonical's time, so I'll probably add an appropriate
copyright notice too; I seem to have failed to do so for earlier work.)

Index: debian/cdebconf-newt-udeb.templates
===================================================================
--- debian/cdebconf-newt-udeb.templates	(revision 59096)
+++ 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: src/template.c
===================================================================
--- src/template.c	(revision 59096)
+++ 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 59096)
+++ 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/newt/cdebconf_newt.h
===================================================================
--- src/modules/frontend/newt/cdebconf_newt.h	(revision 59096)
+++ 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 59096)
+++ 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,21 @@
     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);
+}
+
 /* ----------------------------------------------------------------------- */
 struct question_handlers {
 	const char *type;
@@ -1158,21 +1195,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 59096)
+++ 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 59096)
+++ 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 59096)
+++ 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")

Thanks,

-- 
Colin Watson                                       [cjwatson@debian.org]


Reply to: