>> @@ -234,12 +237,19 @@ static void print_menu(struct menu *m)
>> struct menu_entry *me;
>>
>> clear();
>> - gotoXY(2, 1);
>> - if(m->display) {
>> - __print_entry(m->display);
>> + if (m->display) {
>> + int i;
>> + for (i = 0; i < m->display_lines; i++) {
>> + gotoXY(2, 1 + i);
>> + __print_entry(m->display[i]);
>> + }
>> + m->skip_lines = 0;
>> } else {
>> + gotoXY(2, 1);
>> puts("Menu : ");
>> - puts(m->name);
>> + if (m->name)
>> + puts(m->name);
>> + m->skip_lines = 1;
>> }
>
> We could add this to menu_add():
>
> if (!m->display)
> m->display = xasprintf("Menu : %s", m->name);
>
> Then we wouldn't need the if(m->display) here. Would that simplify the
> code?
Yes. But all title things are go into menu_add_titile, so how about this:
diff --git a/common/menu.c b/common/menu.c
index 1f23e45..636c2b8 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -235,21 +235,12 @@ EXPORT_SYMBOL(menu_set_auto_select);
static void print_menu(struct menu *m)
{
struct menu_entry *me;
+ int i;
clear();
- if (m->display) {
- int i;
- for (i = 0; i < m->display_lines; i++) {
- gotoXY(2, 1 + i);
- __print_entry(m->display[i]);
- }
- m->skip_lines = 0;
- } else {
- gotoXY(2, 1);
- puts("Menu : ");
- if (m->name)
- puts(m->name);
- m->skip_lines = 1;
+ for (i = 0; i < m->display_lines; i++) {
+ gotoXY(2, 1 + i);
+ __print_entry(m->display[i]);
}
list_for_each_entry(me, &m->entries, list) {
@@ -538,8 +529,12 @@ void menu_add_title(struct menu *m, const char *display, char *(*parser)(char *s
int lines = 1;
int i;
- if (!strlen(display))
+ if (!strlen(display)) {
+ m->display_lines = 1;
+ m->display = xzalloc(sizeof(*m->display));
+ m->display[0] = xasprintf("Menu : %s", m->name ? m->name : "");
return;
+ }
src = dst = tmp = xstrdup(display);
/* Count lines and separate single string into multiple strings */
+ remove skip_lines everywhere.
> I haven't really understood why the m->skip_lines is necessary, but I
> think it should be removed by making the if/else above unnecessary.
For proper spacing. But yes, it can be removed with patch above.
>> + for (src = tmp, i = 0; i < lines; i++) {
>> + if (parser)
>> + m->display[i] = parser(src);
>> + else
>> + m->display[i] = xstrdup(src);
>
> What's the reason for running parser() over the separated strings? Can't
> you run parser() over the original multiline string instead?
I wanted to consume more CPU cycles so that the Earth will suffer from power starvation. But you ruined my evil plan, clap-clap-clap.
> What if one of the variables expanded in shell_expand() contains a newline
> character?
It will be incorrect rendering. C u in patch v2 series.
>
>> + /* Go to the next line */
>> + src += strlen(src) + 1;
>> + }
>> +
>> + free(tmp);
>> +}
>> +EXPORT_SYMBOL(menu_add_title);
>> diff --git a/common/menutree.c b/common/menutree.c
>> index eb14da0..e2d364d 100644
>> --- a/common/menutree.c
>> +++ b/common/menutree.c
>> @@ -19,6 +19,7 @@
>> #include <shell.h>
>> #include <libfile.h>
>>
>> +#include <linux/ctype.h>
>> #include <linux/stat.h>
>>
>> struct menutree {
>> @@ -95,6 +96,7 @@ int menutree(const char *path, int toplevel)
>> glob_t g;
>> int i;
>> char *globpath, *display;
>> + size_t size;
>>
>> menu = menu_alloc();
>>
>> @@ -106,14 +108,21 @@ int menutree(const char *path, int toplevel)
>> goto out;
>> }
>>
>> - display = read_file_line("%s/title", path);
>> + globpath = basprintf("%s/title", path);
>> + display = read_file(globpath, &size);
>> + /* Remove trailing whitespaces */
>> + while (size > 0 && isspace(display[size - 1]))
>> + size--;
>
> We have the strim() function for this purpose.
* strim - Removes leading and trailing whitespace from @s.
And mine does only:
>> + /* Remove trailing whitespaces */
Because it's necessary to preserve identation in multiline title so that barebox will be able to display such text:
Barebox boot loader
Board: SomeSuperBoard
CPU: ARMv8 @800MHz
Mem: 512MB
Menu:
....
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
