[1/2] kconfig: menuconfig: simplify global jump key assignment

  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@kernel.org>
---

 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(-)
  

On Thu, Jun 29, 2023 at 12:03 PM Masahiro Yamada <masahiroy@kernel.org> 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@kernel.org>
> ---
>
>  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)

The change for refresh_text_box is very large.
Is there an easy way to split the change of `refresh_text_box` and
everything else
while still maintaining bisectability?

>  {
> -       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)) {

`extra_key_cb` is null when not used, on the help page this will segfault.

if (extra_key_cb && 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;
> +
> +       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,
> -                       .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);
> 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.

Other than the call to null, looks good. After that change.

Reviewed-by: Jesse Taube <Mr.Bossman075@gmail.com>

Thanks,
Jesse Taube

> +{
> +       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
>
  

On Sat, Jul 1, 2023 at 12:58 PM Jesse T <mr.bossman075@gmail.com> wrote:
>
> On Thu, Jun 29, 2023 at 12:03 PM Masahiro Yamada <masahiroy@kernel.org> 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@kernel.org>
> > ---
> >
> >  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)
>
> The change for refresh_text_box is very large.
> Is there an easy way to split the change of `refresh_text_box` and
> everything else
> while still maintaining bisectability?


I do not think the change is large or complicated.

It just stopped passing down 'update_text' and 'data'.
The same pattern.

The point is this is revert of 95ac9b3b585d

The revert should not be split.


> > @@ -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)) {
>
> `extra_key_cb` is null when not used, on the help page this will segfault.


Thanks.
I will fix it.




> > 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?
  

On Thu, Jun 29, 2023 at 12:03 PM Masahiro Yamada <masahiroy@kernel.org> 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@kernel.org>
> ---
>
>  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
>
  

On Sun, Jul 2, 2023 at 3:32 PM Jesse T <mr.bossman075@gmail.com> wrote:
>
> On Thu, Jun 29, 2023 at 12:03 PM Masahiro Yamada <masahiroy@kernel.org> 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@kernel.org>
> > ---
> >
> >  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) {
Sorry forgot to mention this, but start and end should be size_t.
You get -Wsign-compare.

> > -                       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
> >
  

On Sun, Jul 2, 2023 at 4:26 PM Jesse T <mr.bossman075@gmail.com> wrote:
>
> On Sun, Jul 2, 2023 at 3:32 PM Jesse T <mr.bossman075@gmail.com> wrote:
> >
> > On Thu, Jun 29, 2023 at 12:03 PM Masahiro Yamada <masahiroy@kernel.org> 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@kernel.org>
> > > ---
> > >
> > >  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) {
> Sorry forgot to mention this, but start and end should be size_t.
> You get -Wsign-compare.
>
> > > -                       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
> > >

Hi Masahiro Yamada,

Any updates?

Thanks,
Jesse Taube