On Thu, Jun 29, 2023 at 12:03 PM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote: > > Commit 95ac9b3b585d ("menuconfig: Assign jump keys per-page instead > of globally") injects a lot of hacks to the bottom of the textbox > infrastructure. > > I reverted many of them without changing the behavior. (almost) > Now, the key markers are inserted when constructing the search result > instead of updating the text buffer on-the-fly. > > The buffer passed to the textbox got back to a constant string. > The ugly casts from (const char *) to (char *) went away. > > A disadvantage is that the same key numbers might be diplayed multiple > times in the dialog if you use a huge window (but I believe it is > unlikely to happen). > > Signed-off-by: Masahiro Yamada <masahiroy@xxxxxxxxxx> > --- > > scripts/kconfig/lkc.h | 1 + > scripts/kconfig/lxdialog/dialog.h | 10 ++-- > scripts/kconfig/lxdialog/textbox.c | 68 +++++++++-------------- > scripts/kconfig/mconf.c | 86 +++++++++++++++++------------- > scripts/kconfig/menu.c | 22 ++++++-- > 5 files changed, 97 insertions(+), 90 deletions(-) > > diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h > index e7118d62a45f..d5c27180ce91 100644 > --- a/scripts/kconfig/lkc.h > +++ b/scripts/kconfig/lkc.h > @@ -101,6 +101,7 @@ const char *menu_get_prompt(struct menu *menu); > struct menu *menu_get_parent_menu(struct menu *menu); > bool menu_has_help(struct menu *menu); > const char *menu_get_help(struct menu *menu); > +int get_jump_key(void); > struct gstr get_relations_str(struct symbol **sym_arr, struct list_head *head); > void menu_get_ext_help(struct menu *menu, struct gstr *help); > > diff --git a/scripts/kconfig/lxdialog/dialog.h b/scripts/kconfig/lxdialog/dialog.h > index 347daf25fdc8..cd1b59c24b21 100644 > --- a/scripts/kconfig/lxdialog/dialog.h > +++ b/scripts/kconfig/lxdialog/dialog.h > @@ -196,13 +196,9 @@ int first_alpha(const char *string, const char *exempt); > int dialog_yesno(const char *title, const char *prompt, int height, int width); > int dialog_msgbox(const char *title, const char *prompt, int height, > int width, int pause); > - > - > -typedef void (*update_text_fn)(char *buf, size_t start, size_t end, void > - *_data); > -int dialog_textbox(const char *title, char *tbuf, int initial_height, > - int initial_width, int *keys, int *_vscroll, int *_hscroll, > - update_text_fn update_text, void *data); > +int dialog_textbox(const char *title, const char *tbuf, int initial_height, > + int initial_width, int *_vscroll, int *_hscroll, > + int (*extra_key_cb)(int, int, int, void *), void *data); > int dialog_menu(const char *title, const char *prompt, > const void *selected, int *s_scroll); > int dialog_checklist(const char *title, const char *prompt, int height, > diff --git a/scripts/kconfig/lxdialog/textbox.c b/scripts/kconfig/lxdialog/textbox.c > index bc4d4fb1dc75..e6cd7bb83746 100644 > --- a/scripts/kconfig/lxdialog/textbox.c > +++ b/scripts/kconfig/lxdialog/textbox.c > @@ -10,8 +10,8 @@ > > static int hscroll; > static int begin_reached, end_reached, page_length; > -static char *buf; > -static char *page; > +static const char *buf, *page; > +static int start, end; > > /* > * Go back 'n' lines in text. Called by dialog_textbox(). > @@ -98,21 +98,10 @@ static void print_line(WINDOW *win, int row, int width) > /* > * Print a new page of text. > */ > -static void print_page(WINDOW *win, int height, int width, update_text_fn > - update_text, void *data) > +static void print_page(WINDOW *win, int height, int width) > { > int i, passed_end = 0; > > - if (update_text) { > - char *end; > - > - for (i = 0; i < height; i++) > - get_line(); > - end = page; > - back_lines(height); > - update_text(buf, page - buf, end - buf, data); > - } > - > page_length = 0; > for (i = 0; i < height; i++) { > print_line(win, i, width); > @@ -142,24 +131,26 @@ static void print_position(WINDOW *win) > * refresh window content > */ > static void refresh_text_box(WINDOW *dialog, WINDOW *box, int boxh, int boxw, > - int cur_y, int cur_x, update_text_fn update_text, > - void *data) > + int cur_y, int cur_x) > { > - print_page(box, boxh, boxw, update_text, data); > + start = page - buf; > + > + print_page(box, boxh, boxw); > print_position(dialog); > wmove(dialog, cur_y, cur_x); /* Restore cursor position */ > wrefresh(dialog); > + > + end = page - buf; > } > > /* > * Display text from a file in a dialog box. > * > * keys is a null-terminated array > - * update_text() may not add or remove any '\n' or '\0' in tbuf > */ > -int dialog_textbox(const char *title, char *tbuf, int initial_height, > - int initial_width, int *keys, int *_vscroll, int *_hscroll, > - update_text_fn update_text, void *data) > +int dialog_textbox(const char *title, const char *tbuf, int initial_height, > + int initial_width, int *_vscroll, int *_hscroll, > + int (*extra_key_cb)(int, int, int, void *), void *data) > { > int i, x, y, cur_x, cur_y, key = 0; > int height, width, boxh, boxw; > @@ -239,8 +230,7 @@ int dialog_textbox(const char *title, char *tbuf, int initial_height, > > /* Print first page of text */ > attr_clear(box, boxh, boxw, dlg.dialog.atr); > - refresh_text_box(dialog, box, boxh, boxw, cur_y, cur_x, update_text, > - data); > + refresh_text_box(dialog, box, boxh, boxw, cur_y, cur_x); > > while (!done) { > key = wgetch(dialog); > @@ -259,8 +249,7 @@ int dialog_textbox(const char *title, char *tbuf, int initial_height, > begin_reached = 1; > page = buf; > refresh_text_box(dialog, box, boxh, boxw, > - cur_y, cur_x, update_text, > - data); > + cur_y, cur_x); > } > break; > case 'G': /* Last page */ > @@ -270,8 +259,7 @@ int dialog_textbox(const char *title, char *tbuf, int initial_height, > /* point to last char in buf */ > page = buf + strlen(buf); > back_lines(boxh); > - refresh_text_box(dialog, box, boxh, boxw, cur_y, > - cur_x, update_text, data); > + refresh_text_box(dialog, box, boxh, boxw, cur_y, cur_x); > break; > case 'K': /* Previous line */ > case 'k': > @@ -280,8 +268,7 @@ int dialog_textbox(const char *title, char *tbuf, int initial_height, > break; > > back_lines(page_length + 1); > - refresh_text_box(dialog, box, boxh, boxw, cur_y, > - cur_x, update_text, data); > + refresh_text_box(dialog, box, boxh, boxw, cur_y, cur_x); > break; > case 'B': /* Previous page */ > case 'b': > @@ -290,8 +277,7 @@ int dialog_textbox(const char *title, char *tbuf, int initial_height, > if (begin_reached) > break; > back_lines(page_length + boxh); > - refresh_text_box(dialog, box, boxh, boxw, cur_y, > - cur_x, update_text, data); > + refresh_text_box(dialog, box, boxh, boxw, cur_y, cur_x); > break; > case 'J': /* Next line */ > case 'j': > @@ -300,8 +286,7 @@ int dialog_textbox(const char *title, char *tbuf, int initial_height, > break; > > back_lines(page_length - 1); > - refresh_text_box(dialog, box, boxh, boxw, cur_y, > - cur_x, update_text, data); > + refresh_text_box(dialog, box, boxh, boxw, cur_y, cur_x); > break; > case KEY_NPAGE: /* Next page */ > case ' ': > @@ -310,8 +295,7 @@ int dialog_textbox(const char *title, char *tbuf, int initial_height, > break; > > begin_reached = 0; > - refresh_text_box(dialog, box, boxh, boxw, cur_y, > - cur_x, update_text, data); > + refresh_text_box(dialog, box, boxh, boxw, cur_y, cur_x); > break; > case '0': /* Beginning of line */ > case 'H': /* Scroll left */ > @@ -326,8 +310,7 @@ int dialog_textbox(const char *title, char *tbuf, int initial_height, > hscroll--; > /* Reprint current page to scroll horizontally */ > back_lines(page_length); > - refresh_text_box(dialog, box, boxh, boxw, cur_y, > - cur_x, update_text, data); > + refresh_text_box(dialog, box, boxh, boxw, cur_y, cur_x); > break; > case 'L': /* Scroll right */ > case 'l': > @@ -337,8 +320,7 @@ int dialog_textbox(const char *title, char *tbuf, int initial_height, > hscroll++; > /* Reprint current page to scroll horizontally */ > back_lines(page_length); > - refresh_text_box(dialog, box, boxh, boxw, cur_y, > - cur_x, update_text, data); > + refresh_text_box(dialog, box, boxh, boxw, cur_y, cur_x); > break; > case KEY_ESC: > if (on_key_esc(dialog) == KEY_ESC) > @@ -351,11 +333,9 @@ int dialog_textbox(const char *title, char *tbuf, int initial_height, > on_key_resize(); > goto do_resize; > default: > - for (i = 0; keys[i]; i++) { > - if (key == keys[i]) { > - done = true; > - break; > - } > + if (extra_key_cb(key, start, end, data)) { > + done = true; > + break; > } > } > } > diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c > index 53d8834d12fe..7adfd6537279 100644 > --- a/scripts/kconfig/mconf.c > +++ b/scripts/kconfig/mconf.c > @@ -288,6 +288,7 @@ static int single_menu_mode; > static int show_all_options; > static int save_and_exit; > static int silent; > +static int jump_key; > > static void conf(struct menu *menu, struct menu *active_menu); > > @@ -348,19 +349,19 @@ static void reset_subtitle(void) > set_dialog_subtitles(subtitles); > } > > -static int show_textbox_ext(const char *title, char *text, int r, int c, int > - *keys, int *vscroll, int *hscroll, update_text_fn > - update_text, void *data) > +static int show_textbox_ext(const char *title, const char *text, int r, int c, > + int *vscroll, int *hscroll, > + int (*extra_key_cb)(int, int, int, void *), > + void *data) > { > dialog_clear(); > - return dialog_textbox(title, text, r, c, keys, vscroll, hscroll, > - update_text, data); > + return dialog_textbox(title, text, r, c, vscroll, hscroll, > + extra_key_cb, data); > } > > static void show_textbox(const char *title, const char *text, int r, int c) > { > - show_textbox_ext(title, (char *) text, r, c, (int []) {0}, NULL, NULL, > - NULL, NULL); > + show_textbox_ext(title, text, r, c, NULL, NULL, NULL, NULL); > } > > static void show_helptext(const char *title, const char *text) > @@ -381,35 +382,51 @@ static void show_help(struct menu *menu) > > struct search_data { > struct list_head *head; > - struct menu **targets; > - int *keys; > + struct menu *target; > }; > > -static void update_text(char *buf, size_t start, size_t end, void *_data) > +static int next_key(int key) > +{ > + key++; > + > + if (key > '9') > + key = '1'; > + > + return key; > +} > + > +static int handle_search_keys(int key, int start, int end, void *_data) > { > struct search_data *data = _data; > struct jump_key *pos; > - int k = 0; > + > + if (key < '1' || key > '9') > + return 0; > > list_for_each_entry(pos, data->head, entries) { > - if (pos->offset >= start && pos->offset < end) { > - char header[4]; > + if (pos->offset >= start) { > + if (pos->offset >= end) > + break; > > - if (k < JUMP_NB) { > - int key = '0' + (pos->index % JUMP_NB) + 1; > - > - sprintf(header, "(%c)", key); > - data->keys[k] = key; > - data->targets[k] = pos->target; > - k++; > - } else { > - sprintf(header, " "); > + if (key == '1' + (pos->index % JUMP_NB)) { > + data->target = pos->target; > + return 1; > } > - > - memcpy(buf + pos->offset, header, sizeof(header) - 1); > } > } > - data->keys[k] = 0; > + > + return 0; > +} > + > +int get_jump_key(void) > +{ > + int cur_key; > + > + cur_key = jump_key; There should also be a check to see if jump_key is valid. jump_key can be set to 0 and will have weird effects. > + > + jump_key = next_key(cur_key); > + > + return cur_key; > } > > static void search_conf(void) > @@ -456,26 +473,23 @@ static void search_conf(void) > sym_arr = sym_re_search(dialog_input); > do { > LIST_HEAD(head); > - struct menu *targets[JUMP_NB]; > - int keys[JUMP_NB + 1], i; > struct search_data data = { > .head = &head, .target = NULL, We check if target is null later on, we can make it more explicit > - .targets = targets, > - .keys = keys, > }; > struct jump_key *pos, *tmp; > > + jump_key = '1'; > res = get_relations_str(sym_arr, &head); > set_subtitle(); > dres = show_textbox_ext("Search Results", str_get(&res), 0, 0, > - keys, &vscroll, &hscroll, &update_text, > - &data); > + &vscroll, &hscroll, > + handle_search_keys, &data); > again = false; > - for (i = 0; i < JUMP_NB && keys[i]; i++) > - if (dres == keys[i]) { > - conf(targets[i]->parent, targets[i]); > - again = true; > - } > + if (dres >= '1' && dres <= '9') { > + assert(data.target != NULL); > + conf(data.target->parent, data.target); > + again = true; > + } > str_free(&res); > list_for_each_entry_safe(pos, tmp, &head, entries) > free(pos); while here the formatting on the above line is one extra tab indented. > diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c > index b90fff833588..5578b8bc8a23 100644 > --- a/scripts/kconfig/menu.c > +++ b/scripts/kconfig/menu.c > @@ -701,6 +701,11 @@ static void get_dep_str(struct gstr *r, struct expr *expr, const char *prefix) > } > } > > > > +int __attribute__((weak)) get_jump_key(void) > > This seems like a non-optimal solution, otherwise fine. > Do you have a better idea? I do not. > +{ > + return -1; > +} > + > static void get_prompt_str(struct gstr *r, struct property *prop, > struct list_head *head) > { > @@ -743,11 +748,22 @@ static void get_prompt_str(struct gstr *r, struct property *prop, > } > > str_printf(r, " Location:\n"); > - for (j = 4; --i >= 0; j += 2) { > + for (j = 0; --i >= 0; j++) { > + int jk = -1; > + int indent = 2 * j + 4; > + > menu = submenu[i]; > - if (jump && menu == location) > + if (jump && menu == location) { > jump->offset = strlen(r->s); > - str_printf(r, "%*c-> %s", j, ' ', menu_get_prompt(menu)); > + jk = get_jump_key(); > + } > + > + if (jk >= 0) { > + str_printf(r, "(%c)", jk); > + indent -= 3; > + } > + > + str_printf(r, "%*c-> %s", indent, ' ', menu_get_prompt(menu)); > if (menu->sym) { > str_printf(r, " (%s [=%s])", menu->sym->name ? > menu->sym->name : "<choice>", > -- > 2.39.2 >