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

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

[Masahiro Yamada]

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

[PATCH 2/2] kconfig: menuconfig: remove jump_key::index

[Masahiro Yamada]

You do not need to remember the index of each jump key because you can
count it up after a key is pressed.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/kconfig/expr.h  | 1 -
 scripts/kconfig/mconf.c | 7 ++++---
 scripts/kconfig/menu.c  | 8 --------
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index 9c9caca5bd5f..4a9a23b1b7e1 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -275,7 +275,6 @@ struct jump_key {
 	struct list_head entries;
 	size_t offset;
 	struct menu *target;
-	int index;
 };
 
 extern struct file *file_list;
diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
index 7adfd6537279..fcb91d69c774 100644
--- a/scripts/kconfig/mconf.c
+++ b/scripts/kconfig/mconf.c
@@ -22,8 +22,6 @@
 #include "lkc.h"
 #include "lxdialog/dialog.h"
 
-#define JUMP_NB			9
-
 static const char mconf_readme[] =
 "Overview\n"
 "--------\n"
@@ -399,6 +397,7 @@ static int handle_search_keys(int key, int start, int end, void *_data)
 {
 	struct search_data *data = _data;
 	struct jump_key *pos;
+	int index = '1';
 
 	if (key < '1' || key > '9')
 		return 0;
@@ -408,11 +407,13 @@ static int handle_search_keys(int key, int start, int end, void *_data)
 			if (pos->offset >= end)
 				break;
 
-			if (key == '1' + (pos->index % JUMP_NB)) {
+			if (key == index) {
 				data->target = pos->target;
 				return 1;
 			}
 		}
+
+		index = next_key(index);
 	}
 
 	return 0;
diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 5578b8bc8a23..198eb1367e7a 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -735,15 +735,7 @@ static void get_prompt_str(struct gstr *r, struct property *prop,
 	}
 	if (head && location) {
 		jump = xmalloc(sizeof(struct jump_key));
-
 		jump->target = location;
-
-		if (list_empty(head))
-			jump->index = 0;
-		else
-			jump->index = list_entry(head->prev, struct jump_key,
-						 entries)->index + 1;
-
 		list_add_tail(&jump->entries, head);
 	}
 
-- 
2.39.2

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

[Jesse T]

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
>

Re: [PATCH 2/2] kconfig: menuconfig: remove jump_key::index

[Jesse T]

On Thu, Jun 29, 2023 at 12:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> You do not need to remember the index of each jump key because you can
> count it up after a key is pressed.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  scripts/kconfig/expr.h  | 1 -
>  scripts/kconfig/mconf.c | 7 ++++---
>  scripts/kconfig/menu.c  | 8 --------
>  3 files changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
> index 9c9caca5bd5f..4a9a23b1b7e1 100644
> --- a/scripts/kconfig/expr.h
> +++ b/scripts/kconfig/expr.h
> @@ -275,7 +275,6 @@ struct jump_key {
>         struct list_head entries;
>         size_t offset;
>         struct menu *target;
> -       int index;
>  };
>
>  extern struct file *file_list;
> diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
> index 7adfd6537279..fcb91d69c774 100644
> --- a/scripts/kconfig/mconf.c
> +++ b/scripts/kconfig/mconf.c
> @@ -22,8 +22,6 @@
>  #include "lkc.h"
>  #include "lxdialog/dialog.h"
>
> -#define JUMP_NB                        9
> -
>  static const char mconf_readme[] =
>  "Overview\n"
>  "--------\n"
> @@ -399,6 +397,7 @@ static int handle_search_keys(int key, int start, int end, void *_data)
>  {
>         struct search_data *data = _data;
>         struct jump_key *pos;
> +       int index = '1';
>
>         if (key < '1' || key > '9')
>                 return 0;
> @@ -408,11 +407,13 @@ static int handle_search_keys(int key, int start, int end, void *_data)
>                         if (pos->offset >= end)
>                                 break;
>
> -                       if (key == '1' + (pos->index % JUMP_NB)) {
> +                       if (key == index) {
>                                 data->target = pos->target;
>                                 return 1;
>                         }
>                 }
> +
> +               index = next_key(index);
>         }
>
>         return 0;
> diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
> index 5578b8bc8a23..198eb1367e7a 100644
> --- a/scripts/kconfig/menu.c
> +++ b/scripts/kconfig/menu.c
> @@ -735,15 +735,7 @@ static void get_prompt_str(struct gstr *r, struct property *prop,
>         }
>         if (head && location) {
>                 jump = xmalloc(sizeof(struct jump_key));
> -
>                 jump->target = location;
> -
> -               if (list_empty(head))
> -                       jump->index = 0;
> -               else
> -                       jump->index = list_entry(head->prev, struct jump_key,
> -                                                entries)->index + 1;
> -
>                 list_add_tail(&jump->entries, head);
>         }
>
> --
> 2.39.2
>

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

One slight off-topic question.
The names of the menu-based config programs have names similar to their
corresponding file gconfig ('gconf'), xconfig ('qconf'), menuconfig ('mconf'),
and nconfig ('nconf'). The only exceptions to this one-letter naming are mconfig
is not memuconfig and qconfig isn't xconfig. Would it be possible to
add an alias
to fix this?

Side-side note config isn't in the docs.

Thanks,
Jesse Taube

Re: [PATCH 2/2] kconfig: menuconfig: remove jump_key::index

[Randy Dunlap]

Hi Jesse,

On 6/30/23 21:08, Jesse T wrote:
> On Thu, Jun 29, 2023 at 12:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>>
>> You do not need to remember the index of each jump key because you can
>> count it up after a key is pressed.
>>
>> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>> ---

> 
> One slight off-topic question.
> The names of the menu-based config programs have names similar to their
> corresponding file gconfig ('gconf'), xconfig ('qconf'), menuconfig ('mconf'),
> and nconfig ('nconf'). The only exceptions to this one-letter naming are mconfig
> is not memuconfig and qconfig isn't xconfig. Would it be possible to
> add an alias
> to fix this?
> 
> Side-side note config isn't in the docs.

I'm not following what you mean here.
Are you referring to 'make config'?

So: what documentation is missing and where would it be found?

thanks.
-- 
~Randy

Re: [PATCH 2/2] kconfig: menuconfig: remove jump_key::index

[Jesse T]

On Sat, Jul 1, 2023 at 12:23 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> Hi Jesse,
>
> On 6/30/23 21:08, Jesse T wrote:
> > On Thu, Jun 29, 2023 at 12:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >>
> >> You do not need to remember the index of each jump key because you can
> >> count it up after a key is pressed.
> >>
> >> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> >> ---
>
> >
> > One slight off-topic question.
> > The names of the menu-based config programs have names similar to their
> > corresponding file gconfig ('gconf'), xconfig ('qconf'), menuconfig ('mconf'),
> > and nconfig ('nconf'). The only exceptions to this one-letter naming are mconfig
> > is not memuconfig and qconfig isn't xconfig. Would it be possible to
> > add an alias
> > to fix this?
> >
> > Side-side note config isn't in the docs.
>
> I'm not following what you mean here.
> Are you referring to 'make config'?

Typo sorry, `make gconfig`
It's not listed at the top Documentation/kbuild/kconfig.rst and only briefly
mentioned at the bottom.

>
> So: what documentation is missing and where would it be found?
>
> thanks.
> --
> ~Randy

Re: [PATCH 2/2] kconfig: menuconfig: remove jump_key::index

[Randy Dunlap]

On 6/30/23 21:38, Jesse T wrote:
> On Sat, Jul 1, 2023 at 12:23 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>>
>> Hi Jesse,
>>
>> On 6/30/23 21:08, Jesse T wrote:
>>> On Thu, Jun 29, 2023 at 12:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>>>>
>>>> You do not need to remember the index of each jump key because you can
>>>> count it up after a key is pressed.
>>>>
>>>> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
>>>> ---
>>
>>>
>>> One slight off-topic question.
>>> The names of the menu-based config programs have names similar to their
>>> corresponding file gconfig ('gconf'), xconfig ('qconf'), menuconfig ('mconf'),
>>> and nconfig ('nconf'). The only exceptions to this one-letter naming are mconfig
>>> is not memuconfig and qconfig isn't xconfig. Would it be possible to
>>> add an alias
>>> to fix this?
>>>
>>> Side-side note config isn't in the docs.
>>
>> I'm not following what you mean here.
>> Are you referring to 'make config'?
> 
> Typo sorry, `make gconfig`
> It's not listed at the top Documentation/kbuild/kconfig.rst and only briefly
> mentioned at the bottom.
> 

Ah, I see. I don't mind adding it to kconfig.rst, but it is
arguably (IMHO) the least useful of the *config interfaces
since it doesn't have a search capability.

>>
>> So: what documentation is missing and where would it be found?

-- 
~Randy

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

[Masahiro Yamada]

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?





-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/2] kconfig: menuconfig: remove jump_key::index

[Masahiro Yamada]

On Sat, Jul 1, 2023 at 1:09 PM Jesse T <mr.bossman075@gmail.com> wrote:
>
> On Thu, Jun 29, 2023 at 12:03 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > You do not need to remember the index of each jump key because you can
> > count it up after a key is pressed.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> >  scripts/kconfig/expr.h  | 1 -
> >  scripts/kconfig/mconf.c | 7 ++++---
> >  scripts/kconfig/menu.c  | 8 --------
> >  3 files changed, 4 insertions(+), 12 deletions(-)
> >
> > diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
> > index 9c9caca5bd5f..4a9a23b1b7e1 100644
> > --- a/scripts/kconfig/expr.h
> > +++ b/scripts/kconfig/expr.h
> > @@ -275,7 +275,6 @@ struct jump_key {
> >         struct list_head entries;
> >         size_t offset;
> >         struct menu *target;
> > -       int index;
> >  };
> >
> >  extern struct file *file_list;
> > diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
> > index 7adfd6537279..fcb91d69c774 100644
> > --- a/scripts/kconfig/mconf.c
> > +++ b/scripts/kconfig/mconf.c
> > @@ -22,8 +22,6 @@
> >  #include "lkc.h"
> >  #include "lxdialog/dialog.h"
> >
> > -#define JUMP_NB                        9
> > -
> >  static const char mconf_readme[] =
> >  "Overview\n"
> >  "--------\n"
> > @@ -399,6 +397,7 @@ static int handle_search_keys(int key, int start, int end, void *_data)
> >  {
> >         struct search_data *data = _data;
> >         struct jump_key *pos;
> > +       int index = '1';
> >
> >         if (key < '1' || key > '9')
> >                 return 0;
> > @@ -408,11 +407,13 @@ static int handle_search_keys(int key, int start, int end, void *_data)
> >                         if (pos->offset >= end)
> >                                 break;
> >
> > -                       if (key == '1' + (pos->index % JUMP_NB)) {
> > +                       if (key == index) {
> >                                 data->target = pos->target;
> >                                 return 1;
> >                         }
> >                 }
> > +
> > +               index = next_key(index);
> >         }
> >
> >         return 0;
> > diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
> > index 5578b8bc8a23..198eb1367e7a 100644
> > --- a/scripts/kconfig/menu.c
> > +++ b/scripts/kconfig/menu.c
> > @@ -735,15 +735,7 @@ static void get_prompt_str(struct gstr *r, struct property *prop,
> >         }
> >         if (head && location) {
> >                 jump = xmalloc(sizeof(struct jump_key));
> > -
> >                 jump->target = location;
> > -
> > -               if (list_empty(head))
> > -                       jump->index = 0;
> > -               else
> > -                       jump->index = list_entry(head->prev, struct jump_key,
> > -                                                entries)->index + 1;
> > -
> >                 list_add_tail(&jump->entries, head);
> >         }
> >
> > --
> > 2.39.2
> >
>
> Looks good!
> Reviewed-by: Jesse Taube <Mr.Bossman075@gmail.com>
>
> One slight off-topic question.
> The names of the menu-based config programs have names similar to their
> corresponding file gconfig ('gconf'), xconfig ('qconf'), menuconfig ('mconf'),
> and nconfig ('nconf'). The only exceptions to this one-letter naming are mconfig
> is not memuconfig and qconfig isn't xconfig. Would it be possible to
> add an alias
> to fix this?

I also wondered the same question in the past.

I think xconfig was implemented differently at first,
then later it was rewritten based on Qt.
The former developers kept the target name 'xconfig'
to avoid unneeded impacts, but the internal implementation
changed a lot. So, it is a historical reason, I guess.

I do not think it would be rewritten
based on yet another library, but
I just convinced myself "it's just a name" after all.



>
> Side-side note config isn't in the docs.
>
> Thanks,
> Jesse Taube



-- 
Best Regards
Masahiro Yamada

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

[Jesse T]

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
>

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

[Jesse T]

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

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

[Jesse T]

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