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

Bug#508042: newt frontent have inefficient use of screen real estate (multiselect)



tags 508042 patch
thanks

On Sun, Feb 22, 2009 at 06:29:35PM +0200, Oded Naveh wrote:
> It appears that this bug report actually describes two separate bugs.
> 
> # 1. The "... three blank lines between the question text and the
> question itself..."
> # 2. The "... the multiselect part of the question..."  displayed out
> of bounds.

Here are two patches to fix these bugs.
  # 1. cdebconf_loadtemplate.patch
  # 2. cdebconf_newt_need_separate_window.patch

Please find the comments on these patches in the patches themselves.

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 also include the test templates and config I used to test this.
testlong.templates and testlong.config could be dropped in src/test.
When compiled --with-textwrap, debconf produces the expected result for
terminal height from 25 to 40.

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 fix 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 render 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 11:27:44.318953124 +0100
@@ -118,6 +118,7 @@
 typedef int (newt_handler)(struct frontend *obj, struct question *q);
 
 static void newt_progress_stop(struct frontend *obj);
+static char *get_full_description(struct frontend *obj, struct question *q);
 
 #include "cdebconf_newt.h"
 
@@ -275,15 +276,20 @@
 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>/bolean 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;
     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 +304,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 *
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++;
 	}
#!/bin/sh -e
#

. /usr/share/debconf/confmodule

#set -x

db_fset test-multiselect/test seen false
db_input critical test-multiselect/test || true
db_fset test-select/test seen false
db_input critical test-select/test || true
db_fset test-boolean/test seen false
db_input critical test-boolean/test || true
db_fset test-string/test seen false
db_input critical test-string/test || true
db_fset test-note/test seen false
db_input critical test-note/test || true
db_fset test-error/test seen false
db_input critical test-error/test || true
db_go
Template: test-multiselect/title
Type: text
Description: Test multiselect

Template: test-multiselect/test
Type: multiselect
Choices: choice 111, choice 222, choice 333, choice 444
Description: Muliselect description:
 line1 line1 line1 line1 line1 line1 line1 line1
 line2 line2 line2 line2 line2 line2 line2.
 .
 line4 line4 line4 line4 line4 line4 line4 line4
 .
 line6 line6 line6 line6 line6 line6 line6 line6
 .
 line8 line8 line8 line8 line8 line8 line8 line8
 .
 line10 line10 line10 line10 line10 line10 line10 line10
 .
 line12 line12 line12 line12 line12 line12 line12 line12
 .
 line14 line14 line14 line14 line14 line14 line14 line14
 .
 line16 line16 line16 line16 line16 line16 line16 line16

Template: test-select/title
Type: text
Description: Test select

Template: test-select/test
Type: select
Choices: choice1, choice2, choice3, choice4
Description: select description
 line1
 .
 line3
 .
 line5
 .
 line7
 .
 line9
 .
 line11
 .
 line13
 .
 line15

Template: test-boolean/title
Type: text
Description: Test boolean

Template: test-boolean/test
Type: boolean
Description: boolean description
 line1
 .
 line3
 .
 line5
 .
 line7
 .
 line9
 .
 line11
 .
 line13
 .
 line15

Template: test-string/title
Type: text
Description: Test string

Template: test-string/test
Type: string
Description: string description
 line1
 .
 line3
 .
 line5
 .
 line7
 .
 line9
 .
 line11
 .
 line13
 .
 line15

Template: test-note/title
Type: text
Description: Test note

Template: test-note/test
Type: note
Description: note description
 line1
 .
 line3
 .
 line5
 .
 line7
 .
 line9
 .
 line11
 .
 line13
 .
 line15

Template: test-error/title
Type: text
Description: Test error

Template: test-error/test
Type: error
Description: error description
 line1
 .
 line3
 .
 line5
 .
 line7
 .
 line9
 .
 line11
 .
 line13
 .
 line15

// This is a sample debconf configuration file
// Comments can be in C or C++ style, blank lines are ignored
// This file is in a bind-8 like format.

global {
  module_path {
    frontend "../modules/frontend";
    database "../modules/db";
  };

  default {
    frontend "default_fe";
    template "templatedb";
    config "configdb";
  };
};

frontend {
  instance "default_fe" {
    driver "newt";
  };
};

template {
  instance "templatedb" {
    driver "rfc822db";
    path "./templates.dat";
  };
};

config {
  instance "configdb" {
    driver "rfc822db";
    path "./questions.dat";
  };
};


Reply to: