Re: [PATCH] wrap long help lines

Re: [PATCH] wrap long help lines

[Vadim Bendebury (вб)]

Hi Michal, thank you for your comments. Please see my further questions below:

2009/12/11 Michal Marek <mmarek@suse.cz>:
> Vadim Bendebury napsal(a):
>> Help text for certain config options is very extensive (the text includes all
>> config options which the option in question depends on). Long lines are not
>> wrapped, making it impossible to see the list.
>>
>> This patch adds a function to wrap long lines to fit the screen width and
>> makes sure the function is invoked after help text is prepared.
>
> Hi Vadim,
>
> I think that wrapping long lines in menuconfig is a good idea, but I
> have a few problems with your patch.
>

this was just an attempt to get the discussion going  :-)

>> diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
>> index edd3f39..2aa49e8 100644
>> --- a/scripts/kconfig/expr.c
>> +++ b/scripts/kconfig/expr.c
>> @@ -1083,6 +1083,8 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *
>>       }
>>       if (expr_compare_type(prevtoken, e->type) > 0)
>>               fn(data, NULL, ")");
>> +
>> +     str_screen_wrap(data, "||");
>>  }
>
> expr_print() is not the right place to do it. data is a opaque pointer,
> you can't assume that it's a pointer to struct gstr. qconf passes some
> class pointer here, there is code to dump symbol info to a filehandle.
> This should be done in the actual interface program, not in this generic
> code.
>

well, I picked this spot because this is where just the last line of
the text might require wrapping, this makes the wrapping function
simpler.

What do you think of mconf:show_help() before invoking
show_help_text() - is it the right spot to plug this in? Do you have a
suggestion?

> Also, why wrap at "||" but not "&&"? Try the help text for VGA_CONSOLE :).
>

good point. To keep things simple I could invoke the wrapper twice -
with "||" and then  "&&" as the break point string value?


>>  static void expr_print_file_helper(void *data, struct symbol *sym, const char *str)
>> diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
>> index f379b0b..8fc69a3 100644
>> --- a/scripts/kconfig/lkc.h
>> +++ b/scripts/kconfig/lkc.h
>> @@ -112,6 +112,7 @@ struct gstr str_assign(const char *s);
>>  void str_free(struct gstr *gs);
>>  void str_append(struct gstr *gs, const char *s);
>>  void str_printf(struct gstr *gs, const char *fmt, ...);
>> +void str_screen_wrap(struct gstr *gs, const char *break_point);
>>  const char *str_get(struct gstr *gs);
>>
>>  /* symbol.c */
>> diff --git a/scripts/kconfig/util.c b/scripts/kconfig/util.c
>> index b6b2a46..f6c8930 100644
>> --- a/scripts/kconfig/util.c
>> +++ b/scripts/kconfig/util.c
>> @@ -7,6 +7,7 @@
>>
>>  #include <string.h>
>>  #include "lkc.h"
>> +#include <curses.h>
>
> This adds an unnecessary ncurses dependency to the other config interfaces.
>

yes, this did not feel right, the screen width should be passed in as
a parameter.

And this brings the main question: how do we tell the window width in
different modes? Do we care to do this wrap in other than text
terminal modes?


cheers,
/vb

>
>>  /* file already present in list? If not add it */
>>  struct file *file_lookup(const char *name)
>> @@ -131,3 +132,54 @@ const char *str_get(struct gstr *gs)
>>       return gs->s;
>>  }
>>
>> +static const char string_breaker[] = { '\\', '\n' };
>> +#define STRING_BREAKER_SIZE sizeof(string_breaker)
>> +/*
>> + * wrap long lines in the passed in strings. The lines are wrapped to fit into
>> + * the current screen width. The lines can be broken only at 'break points' -
>> + * passed in as the second parameter. A \<cr> (two characters) sequence is
>> + * instered after the appropriate break points to get the line wrapped to fit
>> + * the screen.
>> +*/
>> +void str_screen_wrap(struct gstr *data, const char *break_point)
>> +{
>> +     char *eol_location;
>> +     int total_length;
>> +     int screen_width = getmaxx(stdscr) - 10;
>> +     int break_point_size = strlen(break_point);
>> +
>> +     eol_location = strrchr(data->s, '\n');
>> +     if (!eol_location)
>> +             eol_location = data->s;
>> +
>> +     total_length = strlen(data->s) + 1; /* include trailing zero */
>> +
>> +     /* while last line's length exceeds screen width */
>> +     while ((total_length - (eol_location - data->s) - 1) > screen_width) {
>> +             char *prev_breakp;
>> +             char *breakp = eol_location;
>> +             do {
>> +                     prev_breakp = breakp;
>> +                     breakp = strstr(breakp + 1, break_point);
>> +             } while (breakp &&
>> +                      ((breakp - eol_location) < screen_width));
>> +
>> +             if (prev_breakp == eol_location) {
>> +                     /* no break_points found */
>> +                     return;
>> +             }
>> +
>> +             total_length += STRING_BREAKER_SIZE;
>> +             if (data->len < total_length) {
>> +                     data->s = realloc(data->s, total_length);
>> +                     data->len = total_length;
>> +             }
>> +             prev_breakp += break_point_size;
>> +
>> +             eol_location = prev_breakp + STRING_BREAKER_SIZE;
>> +             /* move the remainder of the string including trailing zero */
>> +             memmove(eol_location, prev_breakp, strlen(prev_breakp) + 1);
>> +             /* insert the line break */
>> +             memcpy(prev_breakp, string_breaker, STRING_BREAKER_SIZE);
>> +     }
>> +}
>
>

Re: [PATCH] wrap long help lines

[Michal Marek]

Vadim Bendebury (вб) napsal(a):
> Hi Michal, thank you for your comments. Please see my further questions below:
> 
> 2009/12/11 Michal Marek <mmarek@suse.cz>:
>> Vadim Bendebury napsal(a):
>>> Help text for certain config options is very extensive (the text includes all
>>> config options which the option in question depends on). Long lines are not
>>> wrapped, making it impossible to see the list.
>>>
>>> This patch adds a function to wrap long lines to fit the screen width and
>>> makes sure the function is invoked after help text is prepared.
>> Hi Vadim,
>>
>> I think that wrapping long lines in menuconfig is a good idea, but I
>> have a few problems with your patch.
>>
> 
> this was just an attempt to get the discussion going  :-)
> 
>>> diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
>>> index edd3f39..2aa49e8 100644
>>> --- a/scripts/kconfig/expr.c
>>> +++ b/scripts/kconfig/expr.c
>>> @@ -1083,6 +1083,8 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *
>>>       }
>>>       if (expr_compare_type(prevtoken, e->type) > 0)
>>>               fn(data, NULL, ")");
>>> +
>>> +     str_screen_wrap(data, "||");
>>>  }
>> expr_print() is not the right place to do it. data is a opaque pointer,
>> you can't assume that it's a pointer to struct gstr. qconf passes some
>> class pointer here, there is code to dump symbol info to a filehandle.
>> This should be done in the actual interface program, not in this generic
>> code.
>>
> 
> well, I picked this spot because this is where just the last line of
> the text might require wrapping, this makes the wrapping function
> simpler.
> 
> What do you think of mconf:show_help() before invoking
> show_help_text() - is it the right spot to plug this in? Do you have a
> suggestion?

If you look at the expr_print() prototype, it takes a pointer to a
callback and a data argument. This callback gets repeatedly called to
store or display parts of the text. So you could create a new callback
that pastes the parts together and inserts a newline if needed. That
way, you don't have to memcpy() the text around afterwards.


> And this brings the main question: how do we tell the window width in
> different modes? Do we care to do this wrap in other than text
> terminal modes?

Qt and Gtk use widgets that take care of proper wrapping. oldconfig just
prints the text on the terminal which does the wrapping (not at word
boundaries, but that's IMO not an issue). So it's really just menuconfig
(and nconfig, but porting your change to nconfig should be trivial).

Michal

Re: [PATCH] wrap long help lines

[Vadim Bendebury (вб)]

2009/12/12 Michal Marek <mmarek@suse.cz>:
>> What do you think of mconf:show_help() before invoking
>> show_help_text() - is it the right spot to plug this in? Do you have a
>> suggestion?>
>
> If you look at the expr_print() prototype, it takes a pointer to a
> callback and a data argument. This callback gets repeatedly called to
> store or display parts of the text. So you could create a new callback
> that pastes the parts together and inserts a newline if needed. That
> way, you don't have to memcpy() the text around afterwards.
>

expr_print() gets  called through expr_gstr_print() from three
different places in menu.c, how would I limit the scope of this change
to the case of help message display only? I think this change should
only affect help messages - don't you agree?

cheers,
/vb

Re: [PATCH] wrap long help lines

[Michal Marek]

On 13.12.2009 04:59, Vadim Bendebury (вб) wrote:
> 2009/12/12 Michal Marek <mmarek@suse.cz>:
>>> What do you think of mconf:show_help() before invoking
>>> show_help_text() - is it the right spot to plug this in? Do you have a
>>> suggestion?>
>>
>> If you look at the expr_print() prototype, it takes a pointer to a
>> callback and a data argument. This callback gets repeatedly called to
>> store or display parts of the text. So you could create a new callback
>> that pastes the parts together and inserts a newline if needed. That
>> way, you don't have to memcpy() the text around afterwards.
>>
> 
> expr_print() gets  called through expr_gstr_print() from three
> different places in menu.c, how would I limit the scope of this change
> to the case of help message display only? I think this change should
> only affect help messages - don't you agree?

AFAICS all three are called from within menu_get_ext_help() only.

Michal

[PATCH] wrap long help lines

[Vadim Bendebury (вб)]

[just in case it messes up some automation resending with corrected
subject string]

Help text for certain config options is very extensive (the text includes all
config options which the option in question depends on). Long lines are not
wrapped, making it impossible to see the list.

This patch adds a function to wrap long lines to fit the screen width and
makes sure the function is invoked after help text is prepared.

Tested by running

mkdir ../build/powerpc
ARCH=powerpc make menuconfig O=../build/powerpc

in the kernel sorce tree. Once menu is displayed, hitting c<cr>cc? brings up
the help message for CONFIG_CRYPTO_MANAGER, now properly wrapped.


Signed-off-by: Vadim Bendebury <vbendeb@gmail.com>
---
 scripts/kconfig/expr.c |    2 +
 scripts/kconfig/lkc.h  |    1 +
 scripts/kconfig/util.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
index edd3f39..2aa49e8 100644
--- a/scripts/kconfig/expr.c
+++ b/scripts/kconfig/expr.c
@@ -1083,6 +1083,8 @@ void expr_print(struct expr *e, void (*fn)(void
*, struct symbol *, const char *
       }
       if (expr_compare_type(prevtoken, e->type) > 0)
               fn(data, NULL, ")");
+
+       str_screen_wrap(data, "||");
 }

 static void expr_print_file_helper(void *data, struct symbol *sym,
const char *str)
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index f379b0b..8fc69a3 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -112,6 +112,7 @@ struct gstr str_assign(const char *s);
 void str_free(struct gstr *gs);
 void str_append(struct gstr *gs, const char *s);
 void str_printf(struct gstr *gs, const char *fmt, ...);
+void str_screen_wrap(struct gstr *gs, const char *break_point);
 const char *str_get(struct gstr *gs);

 /* symbol.c */
diff --git a/scripts/kconfig/util.c b/scripts/kconfig/util.c
index b6b2a46..f6c8930 100644
--- a/scripts/kconfig/util.c
+++ b/scripts/kconfig/util.c
@@ -7,6 +7,7 @@

 #include <string.h>
 #include "lkc.h"
+#include <curses.h>

 /* file already present in list? If not add it */
 struct file *file_lookup(const char *name)
@@ -131,3 +132,54 @@ const char *str_get(struct gstr *gs)
       return gs->s;
 }

+static const char string_breaker[] = { '\\', '\n' };
+#define STRING_BREAKER_SIZE sizeof(string_breaker)
+/*
+ * wrap long lines in the passed in strings. The lines are wrapped to fit into
+ * the current screen width. The lines can be broken only at 'break points' -
+ * passed in as the second parameter. A \<cr> (two characters) sequence is
+ * instered after the appropriate break points to get the line wrapped to fit
+ * the screen.
+*/
+void str_screen_wrap(struct gstr *data, const char *break_point)
+{
+       char *eol_location;
+       int total_length;
+       int screen_width = getmaxx(stdscr) - 10;
+       int break_point_size = strlen(break_point);
+
+       eol_location = strrchr(data->s, '\n');
+       if (!eol_location)
+               eol_location = data->s;
+
+       total_length = strlen(data->s) + 1; /* include trailing zero */
+
+       /* while last line's length exceeds screen width */
+       while ((total_length - (eol_location - data->s) - 1) > screen_width) {
+               char *prev_breakp;
+               char *breakp = eol_location;
+               do {
+                       prev_breakp = breakp;
+                       breakp = strstr(breakp + 1, break_point);
+               } while (breakp &&
+                        ((breakp - eol_location) < screen_width));
+
+               if (prev_breakp == eol_location) {
+                       /* no break_points found */
+                       return;
+               }
+
+               total_length += STRING_BREAKER_SIZE;
+               if (data->len < total_length) {
+                       data->s = realloc(data->s, total_length);
+                       data->len = total_length;
+               }
+               prev_breakp += break_point_size;
+
+               eol_location = prev_breakp + STRING_BREAKER_SIZE;
+               /* move the remainder of the string including trailing zero */
+               memmove(eol_location, prev_breakp, strlen(prev_breakp) + 1);
+               /* insert the line break */
+               memcpy(prev_breakp, string_breaker, STRING_BREAKER_SIZE);
+       }
+}
--
1.5.4.3

[PATCH} wrap long help lines

[Vadim Bendebury]

Help text for certain config options is very extensive (the text includes all
config options which the option in question depends on). Long lines are not
wrapped, making it impossible to see the list.

This patch adds a function to wrap long lines to fit the screen width and
makes sure the function is invoked after help text is prepared.

Tested by running

mkdir ../build/powerpc
ARCH=powerpc make menuconfig O=../build/powerpc

in the kernel sorce tree. Once menu is displayed, hitting c<cr>cc? brings up
the help message for CONFIG_CRYPTO_MANAGER, now properly wrapped.


Signed-off-by: Vadim Bendebury <vbendeb@google.com>
---
 scripts/kconfig/expr.c |    2 +
 scripts/kconfig/lkc.h  |    1 +
 scripts/kconfig/util.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 0 deletions(-)

diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
index edd3f39..2aa49e8 100644
--- a/scripts/kconfig/expr.c
+++ b/scripts/kconfig/expr.c
@@ -1083,6 +1083,8 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *
 	}
 	if (expr_compare_type(prevtoken, e->type) > 0)
 		fn(data, NULL, ")");
+
+	str_screen_wrap(data, "||");
 }
 
 static void expr_print_file_helper(void *data, struct symbol *sym, const char *str)
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index f379b0b..8fc69a3 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -112,6 +112,7 @@ struct gstr str_assign(const char *s);
 void str_free(struct gstr *gs);
 void str_append(struct gstr *gs, const char *s);
 void str_printf(struct gstr *gs, const char *fmt, ...);
+void str_screen_wrap(struct gstr *gs, const char *break_point);
 const char *str_get(struct gstr *gs);
 
 /* symbol.c */
diff --git a/scripts/kconfig/util.c b/scripts/kconfig/util.c
index b6b2a46..f6c8930 100644
--- a/scripts/kconfig/util.c
+++ b/scripts/kconfig/util.c
@@ -7,6 +7,7 @@
 
 #include <string.h>
 #include "lkc.h"
+#include <curses.h>
 
 /* file already present in list? If not add it */
 struct file *file_lookup(const char *name)
@@ -131,3 +132,54 @@ const char *str_get(struct gstr *gs)
 	return gs->s;
 }
 
+static const char string_breaker[] = { '\\', '\n' };
+#define STRING_BREAKER_SIZE sizeof(string_breaker)
+/*
+ * wrap long lines in the passed in strings. The lines are wrapped to fit into
+ * the current screen width. The lines can be broken only at 'break points' -
+ * passed in as the second parameter. A \<cr> (two characters) sequence is
+ * instered after the appropriate break points to get the line wrapped to fit
+ * the screen.
+*/
+void str_screen_wrap(struct gstr *data, const char *break_point)
+{
+	char *eol_location;
+	int total_length;
+	int screen_width = getmaxx(stdscr) - 10;
+	int break_point_size = strlen(break_point);
+
+	eol_location = strrchr(data->s, '\n');
+	if (!eol_location)
+		eol_location = data->s;
+
+	total_length = strlen(data->s) + 1; /* include trailing zero */
+
+	/* while last line's length exceeds screen width */
+	while ((total_length - (eol_location - data->s) - 1) > screen_width) {
+		char *prev_breakp;
+		char *breakp = eol_location;
+		do {
+			prev_breakp = breakp;
+			breakp = strstr(breakp + 1, break_point);
+		} while (breakp &&
+			 ((breakp - eol_location) < screen_width));
+
+		if (prev_breakp == eol_location) {
+			/* no break_points found */
+			return;
+		}
+
+		total_length += STRING_BREAKER_SIZE;
+		if (data->len < total_length) {
+			data->s = realloc(data->s, total_length);
+			data->len = total_length;
+		}
+		prev_breakp += break_point_size;
+
+		eol_location = prev_breakp + STRING_BREAKER_SIZE;
+		/* move the remainder of the string including trailing zero */
+		memmove(eol_location, prev_breakp, strlen(prev_breakp) + 1);
+		/* insert the line break */
+		memcpy(prev_breakp, string_breaker, STRING_BREAKER_SIZE);
+	}
+}
-- 
1.5.4.3

Re: [PATCH} wrap long help lines

[Randy Dunlap]

On Thu, 10 Dec 2009 08:36:21 -0800 (PST) Vadim Bendebury wrote:

> Help text for certain config options is very extensive (the text includes all
> config options which the option in question depends on). Long lines are not
> wrapped, making it impossible to see the list.

It's possible to see the list by using the right arrow key to
scroll/pan the screen side-to-side.
But I'm not objecting to the patch.

> This patch adds a function to wrap long lines to fit the screen width and
> makes sure the function is invoked after help text is prepared.
> 
> Tested by running
> 
> mkdir ../build/powerpc
> ARCH=powerpc make menuconfig O=../build/powerpc
> 
> in the kernel sorce tree. Once menu is displayed, hitting c<cr>cc? brings up
> the help message for CONFIG_CRYPTO_MANAGER, now properly wrapped.
> 
> 
> Signed-off-by: Vadim Bendebury <vbendeb@google.com>
> ---
>  scripts/kconfig/expr.c |    2 +
>  scripts/kconfig/lkc.h  |    1 +
>  scripts/kconfig/util.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 55 insertions(+), 0 deletions(-)
> 
> diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
> index edd3f39..2aa49e8 100644
> --- a/scripts/kconfig/expr.c
> +++ b/scripts/kconfig/expr.c
> @@ -1083,6 +1083,8 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *
>  	}
>  	if (expr_compare_type(prevtoken, e->type) > 0)
>  		fn(data, NULL, ")");
> +
> +	str_screen_wrap(data, "||");
>  }
>  
>  static void expr_print_file_helper(void *data, struct symbol *sym, const char *str)
> diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
> index f379b0b..8fc69a3 100644
> --- a/scripts/kconfig/lkc.h
> +++ b/scripts/kconfig/lkc.h
> @@ -112,6 +112,7 @@ struct gstr str_assign(const char *s);
>  void str_free(struct gstr *gs);
>  void str_append(struct gstr *gs, const char *s);
>  void str_printf(struct gstr *gs, const char *fmt, ...);
> +void str_screen_wrap(struct gstr *gs, const char *break_point);
>  const char *str_get(struct gstr *gs);
>  
>  /* symbol.c */
> diff --git a/scripts/kconfig/util.c b/scripts/kconfig/util.c
> index b6b2a46..f6c8930 100644
> --- a/scripts/kconfig/util.c
> +++ b/scripts/kconfig/util.c
> @@ -7,6 +7,7 @@
>  
>  #include <string.h>
>  #include "lkc.h"
> +#include <curses.h>
>  
>  /* file already present in list? If not add it */
>  struct file *file_lookup(const char *name)
> @@ -131,3 +132,54 @@ const char *str_get(struct gstr *gs)
>  	return gs->s;
>  }
>  
> +static const char string_breaker[] = { '\\', '\n' };
> +#define STRING_BREAKER_SIZE sizeof(string_breaker)
> +/*
> + * wrap long lines in the passed in strings. The lines are wrapped to fit into
> + * the current screen width. The lines can be broken only at 'break points' -
> + * passed in as the second parameter. A \<cr> (two characters) sequence is
> + * instered after the appropriate break points to get the line wrapped to fit
> + * the screen.
> +*/
> +void str_screen_wrap(struct gstr *data, const char *break_point)
> +{
> +	char *eol_location;
> +	int total_length;
> +	int screen_width = getmaxx(stdscr) - 10;
> +	int break_point_size = strlen(break_point);
> +
> +	eol_location = strrchr(data->s, '\n');
> +	if (!eol_location)
> +		eol_location = data->s;
> +
> +	total_length = strlen(data->s) + 1; /* include trailing zero */
> +
> +	/* while last line's length exceeds screen width */
> +	while ((total_length - (eol_location - data->s) - 1) > screen_width) {
> +		char *prev_breakp;
> +		char *breakp = eol_location;
> +		do {
> +			prev_breakp = breakp;
> +			breakp = strstr(breakp + 1, break_point);
> +		} while (breakp &&
> +			 ((breakp - eol_location) < screen_width));
> +
> +		if (prev_breakp == eol_location) {
> +			/* no break_points found */
> +			return;
> +		}
> +
> +		total_length += STRING_BREAKER_SIZE;
> +		if (data->len < total_length) {
> +			data->s = realloc(data->s, total_length);
> +			data->len = total_length;
> +		}
> +		prev_breakp += break_point_size;
> +
> +		eol_location = prev_breakp + STRING_BREAKER_SIZE;
> +		/* move the remainder of the string including trailing zero */
> +		memmove(eol_location, prev_breakp, strlen(prev_breakp) + 1);
> +		/* insert the line break */
> +		memcpy(prev_breakp, string_breaker, STRING_BREAKER_SIZE);
> +	}
> +}
> -- 


---
~Randy

Re: [PATCH} wrap long help lines

[Vadim Bendebury (вб)]

Ah, I did not know that the arrow keys worked, thank you for pointing
that out. But it is still so much more comfortable to see the entire
help text on one screen.

cheers,
/vb


On Thu, Dec 10, 2009 at 8:57 AM, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> On Thu, 10 Dec 2009 08:36:21 -0800 (PST) Vadim Bendebury wrote:
>
>> Help text for certain config options is very extensive (the text includes all
>> config options which the option in question depends on). Long lines are not
>> wrapped, making it impossible to see the list.
>
> It's possible to see the list by using the right arrow key to
> scroll/pan the screen side-to-side.
> But I'm not objecting to the patch.
>
>> This patch adds a function to wrap long lines to fit the screen width and
>> makes sure the function is invoked after help text is prepared.
>>
>> Tested by running
>>
>> mkdir ../build/powerpc
>> ARCH=powerpc make menuconfig O=../build/powerpc
>>
>> in the kernel sorce tree. Once menu is displayed, hitting c<cr>cc? brings up
>> the help message for CONFIG_CRYPTO_MANAGER, now properly wrapped.
>>
>>
>> Signed-off-by: Vadim Bendebury <vbendeb@google.com>
>> ---
>>  scripts/kconfig/expr.c |    2 +
>>  scripts/kconfig/lkc.h  |    1 +
>>  scripts/kconfig/util.c |   52 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 55 insertions(+), 0 deletions(-)
>>
>> diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
>> index edd3f39..2aa49e8 100644
>> --- a/scripts/kconfig/expr.c
>> +++ b/scripts/kconfig/expr.c
>> @@ -1083,6 +1083,8 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *
>>       }
>>       if (expr_compare_type(prevtoken, e->type) > 0)
>>               fn(data, NULL, ")");
>> +
>> +     str_screen_wrap(data, "||");
>>  }
>>
>>  static void expr_print_file_helper(void *data, struct symbol *sym, const char *str)
>> diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
>> index f379b0b..8fc69a3 100644
>> --- a/scripts/kconfig/lkc.h
>> +++ b/scripts/kconfig/lkc.h
>> @@ -112,6 +112,7 @@ struct gstr str_assign(const char *s);
>>  void str_free(struct gstr *gs);
>>  void str_append(struct gstr *gs, const char *s);
>>  void str_printf(struct gstr *gs, const char *fmt, ...);
>> +void str_screen_wrap(struct gstr *gs, const char *break_point);
>>  const char *str_get(struct gstr *gs);
>>
>>  /* symbol.c */
>> diff --git a/scripts/kconfig/util.c b/scripts/kconfig/util.c
>> index b6b2a46..f6c8930 100644
>> --- a/scripts/kconfig/util.c
>> +++ b/scripts/kconfig/util.c
>> @@ -7,6 +7,7 @@
>>
>>  #include <string.h>
>>  #include "lkc.h"
>> +#include <curses.h>
>>
>>  /* file already present in list? If not add it */
>>  struct file *file_lookup(const char *name)
>> @@ -131,3 +132,54 @@ const char *str_get(struct gstr *gs)
>>       return gs->s;
>>  }
>>
>> +static const char string_breaker[] = { '\\', '\n' };
>> +#define STRING_BREAKER_SIZE sizeof(string_breaker)
>> +/*
>> + * wrap long lines in the passed in strings. The lines are wrapped to fit into
>> + * the current screen width. The lines can be broken only at 'break points' -
>> + * passed in as the second parameter. A \<cr> (two characters) sequence is
>> + * instered after the appropriate break points to get the line wrapped to fit
>> + * the screen.
>> +*/
>> +void str_screen_wrap(struct gstr *data, const char *break_point)
>> +{
>> +     char *eol_location;
>> +     int total_length;
>> +     int screen_width = getmaxx(stdscr) - 10;
>> +     int break_point_size = strlen(break_point);
>> +
>> +     eol_location = strrchr(data->s, '\n');
>> +     if (!eol_location)
>> +             eol_location = data->s;
>> +
>> +     total_length = strlen(data->s) + 1; /* include trailing zero */
>> +
>> +     /* while last line's length exceeds screen width */
>> +     while ((total_length - (eol_location - data->s) - 1) > screen_width) {
>> +             char *prev_breakp;
>> +             char *breakp = eol_location;
>> +             do {
>> +                     prev_breakp = breakp;
>> +                     breakp = strstr(breakp + 1, break_point);
>> +             } while (breakp &&
>> +                      ((breakp - eol_location) < screen_width));
>> +
>> +             if (prev_breakp == eol_location) {
>> +                     /* no break_points found */
>> +                     return;
>> +             }
>> +
>> +             total_length += STRING_BREAKER_SIZE;
>> +             if (data->len < total_length) {
>> +                     data->s = realloc(data->s, total_length);
>> +                     data->len = total_length;
>> +             }
>> +             prev_breakp += break_point_size;
>> +
>> +             eol_location = prev_breakp + STRING_BREAKER_SIZE;
>> +             /* move the remainder of the string including trailing zero */
>> +             memmove(eol_location, prev_breakp, strlen(prev_breakp) + 1);
>> +             /* insert the line break */
>> +             memcpy(prev_breakp, string_breaker, STRING_BREAKER_SIZE);
>> +     }
>> +}
>> --
>
>
> ---
> ~Randy
>

Re: [PATCH} wrap long help lines

[Michal Marek]

Vadim Bendebury napsal(a):
> Help text for certain config options is very extensive (the text includes all
> config options which the option in question depends on). Long lines are not
> wrapped, making it impossible to see the list.
> 
> This patch adds a function to wrap long lines to fit the screen width and
> makes sure the function is invoked after help text is prepared.

Hi Vadim,

I think that wrapping long lines in menuconfig is a good idea, but I
have a few problems with your patch.

> diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
> index edd3f39..2aa49e8 100644
> --- a/scripts/kconfig/expr.c
> +++ b/scripts/kconfig/expr.c
> @@ -1083,6 +1083,8 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *
>  	}
>  	if (expr_compare_type(prevtoken, e->type) > 0)
>  		fn(data, NULL, ")");
> +
> +	str_screen_wrap(data, "||");
>  }

expr_print() is not the right place to do it. data is a opaque pointer,
you can't assume that it's a pointer to struct gstr. qconf passes some
class pointer here, there is code to dump symbol info to a filehandle.
This should be done in the actual interface program, not in this generic
code.

Also, why wrap at "||" but not "&&"? Try the help text for VGA_CONSOLE :).

>  static void expr_print_file_helper(void *data, struct symbol *sym, const char *str)
> diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
> index f379b0b..8fc69a3 100644
> --- a/scripts/kconfig/lkc.h
> +++ b/scripts/kconfig/lkc.h
> @@ -112,6 +112,7 @@ struct gstr str_assign(const char *s);
>  void str_free(struct gstr *gs);
>  void str_append(struct gstr *gs, const char *s);
>  void str_printf(struct gstr *gs, const char *fmt, ...);
> +void str_screen_wrap(struct gstr *gs, const char *break_point);
>  const char *str_get(struct gstr *gs);
>  
>  /* symbol.c */
> diff --git a/scripts/kconfig/util.c b/scripts/kconfig/util.c
> index b6b2a46..f6c8930 100644
> --- a/scripts/kconfig/util.c
> +++ b/scripts/kconfig/util.c
> @@ -7,6 +7,7 @@
>  
>  #include <string.h>
>  #include "lkc.h"
> +#include <curses.h>

This adds an unnecessary ncurses dependency to the other config interfaces.


>  /* file already present in list? If not add it */
>  struct file *file_lookup(const char *name)
> @@ -131,3 +132,54 @@ const char *str_get(struct gstr *gs)
>  	return gs->s;
>  }
>  
> +static const char string_breaker[] = { '\\', '\n' };
> +#define STRING_BREAKER_SIZE sizeof(string_breaker)
> +/*
> + * wrap long lines in the passed in strings. The lines are wrapped to fit into
> + * the current screen width. The lines can be broken only at 'break points' -
> + * passed in as the second parameter. A \<cr> (two characters) sequence is
> + * instered after the appropriate break points to get the line wrapped to fit
> + * the screen.
> +*/
> +void str_screen_wrap(struct gstr *data, const char *break_point)
> +{
> +	char *eol_location;
> +	int total_length;
> +	int screen_width = getmaxx(stdscr) - 10;
> +	int break_point_size = strlen(break_point);
> +
> +	eol_location = strrchr(data->s, '\n');
> +	if (!eol_location)
> +		eol_location = data->s;
> +
> +	total_length = strlen(data->s) + 1; /* include trailing zero */
> +
> +	/* while last line's length exceeds screen width */
> +	while ((total_length - (eol_location - data->s) - 1) > screen_width) {
> +		char *prev_breakp;
> +		char *breakp = eol_location;
> +		do {
> +			prev_breakp = breakp;
> +			breakp = strstr(breakp + 1, break_point);
> +		} while (breakp &&
> +			 ((breakp - eol_location) < screen_width));
> +
> +		if (prev_breakp == eol_location) {
> +			/* no break_points found */
> +			return;
> +		}
> +
> +		total_length += STRING_BREAKER_SIZE;
> +		if (data->len < total_length) {
> +			data->s = realloc(data->s, total_length);
> +			data->len = total_length;
> +		}
> +		prev_breakp += break_point_size;
> +
> +		eol_location = prev_breakp + STRING_BREAKER_SIZE;
> +		/* move the remainder of the string including trailing zero */
> +		memmove(eol_location, prev_breakp, strlen(prev_breakp) + 1);
> +		/* insert the line break */
> +		memcpy(prev_breakp, string_breaker, STRING_BREAKER_SIZE);
> +	}
> +}