Bug#508042: newt frontent have inefficient use of screen real estate (multiselect)
On Thu, Feb 26, 2009 at 02:46:46PM +0100, Nicolas François wrote:
>
> There is probably still some space that could be saved for select (not
> multiselect) because 2 lines are reserved for a button, which is not
> displayed if the text and select fits on one window.
> (i.e. when show_select_window(obj, q, 1) is called the <Continue> button
> is not displayed, but some place is reserved for it, as when
> show_select_window(obj, q, 0) is called)
I could fix that one too with cdebconf_newt_reduce_space_select.patch
(it needs cdebconf_newt_need_separate_window.patch)
So here are the 3 patches
* cdebconf_loadtemplate.patch
* cdebconf_newt_need_separate_window.patch
* cdebconf_newt_reduce_space_select.patch
Best Regards,
--
Nekral
If a description ends with a verbatim block (asis == 1), remove_newlines()
adds a newline instead of removing the trailing newline.
This causes the debconf frontend(s?) to display 3 blank lines, instead of
one, between the long description and the short description.
The end of string must be tested before asis.
* src/template.c (remove_newlines): Fix the removal of trailing
newline for descriptions ending with a verbatim block.
diff -aruN ../orig/cdebconf-0.139/src/template.c ./cdebconf-0.139/src/template.c
--- ../orig/cdebconf-0.139/src/template.c 2008-12-26 14:46:33.000000000 +0100
+++ ./cdebconf-0.139/src/template.c 2009-02-26 10:31:41.214635082 +0100
@@ -619,17 +619,14 @@
in+=2;
asis=0;
}
+ else if (*(in+1) == 0)
+ *out = 0;
else if (*(in+1) == ' ')
asis=1;
else if (asis)
asis=0;
else
- {
- if (*(in+1) != 0)
- *out = ' ';
- else
- *out = 0;
- }
+ *out = ' ';
}
out++;
}
This fixes the computation and usage of the window's height. If the height
is not computed correctly, the newt frontend may decide to put the input
box and the explanatory text on the same window, but the text will overlap
on the input box, which renders the input impossible.
#508042 is an example for a multiselect, although I think it could appear
with a select window (not for string and password windows because I think
the input field is always forced to be on the same window).
* src/modules/frontend/newt/newt.c (min_window_height): Document
the computation of the window's height.
* src/modules/frontend/newt/newt.c (min_window_height): Use the
full description instead of the extended description when
possible. This should make min_window_height() more independent
from the get_full_description() internals.
* src/modules/frontend/newt/newt.c (min_window_height): The height
of the text needs to be added to the original size (decoration &
buttons).
* src/modules/frontend/newt/newt.c (need_separate_window):
Document why we subtract 5 for the comparison of the window's
height.
* src/modules/frontend/newt/newt.c (need_separate_window): Do not
use an extra window if the text fits exactly
(min_window_height() == height-5)
diff -aruN ../orig/cdebconf-0.139/src/modules/frontend/newt/newt.c ./cdebconf-0.139/src/modules/frontend/newt/newt.c
--- ../orig/cdebconf-0.139/src/modules/frontend/newt/newt.c 2009-02-13 17:43:37.000000000 +0100
+++ ./cdebconf-0.139/src/modules/frontend/newt/newt.c 2009-02-26 16:17:12.907008645 +0100
@@ -119,6 +119,9 @@
static void newt_progress_stop(struct frontend *obj);
+/* Result must be freed by the caller */
+static char *get_full_description(struct frontend *obj, struct question *q);
+
#include "cdebconf_newt.h"
/* Padding of title width, allows for leading "[!!] " before title
@@ -275,15 +278,22 @@
static int
min_window_height(struct frontend *obj, struct question *q, int win_width)
{
+ // start with a blank or description (note and error)
+ // End with <Continue>/boolean buttons + blank
int height = 3;
char *type = q->template->type;
- char *q_ext_text;
+ char *q_text;
- q_ext_text = q_get_extended_description(obj, q);
- if (q_ext_text != NULL)
- height = cdebconf_newt_get_text_height(q_ext_text, win_width) + 1;
+ if (strcmp(q->template->type, "note") == 0 || strcmp(q->template->type, "error") == 0)
+ q_text = q_get_extended_description(obj, q);
+ else
+ q_text = get_full_description(obj, q);
+ if (q_text != NULL) {
+ height += cdebconf_newt_get_text_height(q_text, win_width) + 1;
+ free (q_text);
+ }
if (strcmp(type, "multiselect") == 0 || strcmp(type, "select") == 0)
- height += 4; // at least three lines for choices + blank line
+ height += 4; // x lines for choices + blank line
else if (strcmp(type, "string") == 0 || strcmp(type, "password") == 0)
height += 2; // input line + blank line
// the others don't need more space
@@ -298,7 +308,8 @@
newtGetScreenSize(&width, &height);
x = min_window_height(obj, q, width-7);
- return (x >= height-5);
+ return (x > height-5);
+ // 5: blue border + title + bottom frame + shadow + menu
}
static char *
The newt frontend does no use the window size efficiently.
Two lines are lost at the bottom of the window when the description and
select are on the same windows (reserved for a <Go Back> button which does
not exist in this case), and one line is lost on the top of the select
windows when the description and the select are on separate windows (the
description is considered zero-length in that case, which causes two blank
lines on the top around this zero-length description).
* src/modules/frontend/newt/newt.c (show_select_window): Use space
more efficiently.
--- ../../work.508042/cdebconf-0.139/src/modules/frontend/newt/newt.c 2009-02-26 20:32:42.506625084 +0100
+++ src/modules/frontend/newt/newt.c 2009-02-26 22:30:10.588470305 +0100
@@ -129,6 +129,14 @@
*/
#define TITLE_PADDING 9
+/* At least one blue line on top
+ * One line for the title / top frame
+ * One line for the bottom frame
+ * One line for shadow on the bottom
+ * One line with a menu
+ */
+#define MIN_DECORATION_HEIGHT 5
+
/* gettext would be much nicer :-( */
static const char *
continue_text(struct frontend *obj)
@@ -292,8 +300,10 @@
height += cdebconf_newt_get_text_height(q_text, win_width) + 1;
free (q_text);
}
- if (strcmp(type, "multiselect") == 0 || strcmp(type, "select") == 0)
+ if (strcmp(type, "multiselect") == 0)
height += 4; // at least three lines for choices + blank line
+ else if (strcmp(type, "select") == 0)
+ height += 2; // as multiselect, but without a button and blank line
else if (strcmp(type, "string") == 0 || strcmp(type, "password") == 0)
height += 2; // input line + blank line
// the others don't need more space
@@ -308,8 +318,7 @@
newtGetScreenSize(&width, &height);
x = min_window_height(obj, q, width-7);
- return (x > height-5);
- // 5: blue border + title + bottom frame + shadow + menu
+ return (x > height-MIN_DECORATION_HEIGHT);
}
static char *
@@ -687,6 +696,8 @@
int listflags = NEWT_FLAG_RETURNEXIT;
int width = 80, height = 24;
int win_width, win_height = -1, t_height, t_width, sel_height, sel_width;
+ int select_list_top;
+ int b_height;
int t_width_title, t_width_buttons;
char **choices, **choices_trans, *defval;
int count = 0, i, ret, defchoice = -1;
@@ -766,18 +777,26 @@
t_height = newtTextboxGetNumLines(textbox);
newtTextboxSetHeight(textbox, t_height);
newtFormAddComponent(form, textbox);
- } else
+ b_height = 0; // A <Go Back> button is not necessary
+ select_list_top = 1+t_height+1;
+ } else {
t_height = 0;
+ b_height = 1;
+ select_list_top = 1; // No description. Only insert a blank line.
+ }
free(full_description);
- win_height = t_height + 5 + sel_height;
- if (win_height > height-5) {
- win_height = height-5;
- sel_height = win_height - t_height - 5;
+ win_height = t_height + sel_height + b_height;
+ // 3 == First blank line + blanks before and after select
+ // or 3 == Blanks before and after select + blank after <Go Back>
+ win_height += 3;
+ if (win_height > height-MIN_DECORATION_HEIGHT) {
+ win_height = height-MIN_DECORATION_HEIGHT;
+ sel_height = win_height - t_height - 3 - b_height;
}
if (count > sel_height)
listflags |= NEWT_FLAG_SCROLL;
cdebconf_newt_create_window(win_width, win_height, obj->title, q->priority);
- listbox = newtListbox((win_width-sel_width-3)/2, 1+t_height+1, sel_height, listflags);
+ listbox = newtListbox((win_width-sel_width-3)/2, select_list_top, sel_height, listflags);
defval = (char *)question_getvalue(q, "");
for (i = 0; i < count; i++) {
newtListboxAppendEntry(listbox, choices_trans[i], choices[tindex[i]]);
Reply to: