diff mbox series

[v1] scripts: kconfig: nconf: Add search jump feature

Message ID 20230618001646.1228521-1-Mr.Bossman075@gmail.com (mailing list archive)
State Superseded
Headers show
Series [v1] scripts: kconfig: nconf: Add search jump feature | expand

Commit Message

Jesse T June 18, 2023, 12:16 a.m. UTC
Menuconfig has a feature where you can "press the key in the (#) prefix
to jump directly to that location. You will be returned to the current
search results after exiting this new menu."

This commit adds this feature to nconfig, with almost identical code.

Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
---
 scripts/kconfig/nconf.c     | 79 +++++++++++++++++++++++++++++++++++--
 scripts/kconfig/nconf.gui.c | 31 ++++++++++++---
 scripts/kconfig/nconf.h     |  5 +++
 3 files changed, 106 insertions(+), 9 deletions(-)

Comments

Masahiro Yamada June 24, 2023, 8:11 a.m. UTC | #1
On Sun, Jun 18, 2023 at 9:16 AM Jesse Taube <mr.bossman075@gmail.com> wrote:
>
> Menuconfig has a feature where you can "press the key in the (#) prefix
> to jump directly to that location. You will be returned to the current
> search results after exiting this new menu."
>
> This commit adds this feature to nconfig, with almost identical code.
>
> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>


Setting the code duplication aside,
does this patch work correctly?



$ make defconfig
$ make nconfig

Press F8

Input "MODULES" in the search box.

You will see:

(1) -> Enable loadable module support

Press the "1" key.


It will navigate to "General setup"
instead of "Enable loadable module support".
Jesse T June 24, 2023, 8:36 p.m. UTC | #2
On Sat, Jun 24, 2023 at 4:11 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Sun, Jun 18, 2023 at 9:16 AM Jesse Taube <mr.bossman075@gmail.com> wrote:
> >
> > Menuconfig has a feature where you can "press the key in the (#) prefix
> > to jump directly to that location. You will be returned to the current
> > search results after exiting this new menu."
> >
> > This commit adds this feature to nconfig, with almost identical code.
> >
> > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
>
>
> Setting the code duplication aside,


If it does function as expected will you accept the patch?
Should I mark the copied codes'  original location?

> does this patch work correctly?
>
>
>
>
> $ make defconfig
> $ make nconfig
>
> Press F8
>
> Input "MODULES" in the search box.
>
> You will see:
>
> (1) -> Enable loadable module support
>
> Press the "1" key.
>
>
> It will navigate to "General setup"
> instead of "Enable loadable module support".


Hmm, this is a confusing issue.
It will take you to the parent menu of the option, it should move you
to the parent option and
move the cursor to the option location. I will see if I can resolve
this oversight.

Thanks,
Jesse Taube

>
>
>
>
>
>
>
> --
> Best Regards
> Masahiro Yamada
Masahiro Yamada June 27, 2023, 12:37 a.m. UTC | #3
On Sun, Jun 25, 2023 at 5:36 AM Jesse T <mr.bossman075@gmail.com> wrote:
>
> On Sat, Jun 24, 2023 at 4:11 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Sun, Jun 18, 2023 at 9:16 AM Jesse Taube <mr.bossman075@gmail.com> wrote:
> > >
> > > Menuconfig has a feature where you can "press the key in the (#) prefix
> > > to jump directly to that location. You will be returned to the current
> > > search results after exiting this new menu."
> > >
> > > This commit adds this feature to nconfig, with almost identical code.
> > >
> > > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> >
> >
> > Setting the code duplication aside,
>
>
> If it does function as expected will you accept the patch?


Probably.

The menuconfig code you copied is really ugly.
Actually, I was refactoring the search jump code a few
months ago, but I did not get around to completing it.








> Should I mark the copied codes'  original location?
>
> > does this patch work correctly?
> >
> >
> >
> >
> > $ make defconfig
> > $ make nconfig
> >
> > Press F8
> >
> > Input "MODULES" in the search box.
> >
> > You will see:
> >
> > (1) -> Enable loadable module support
> >
> > Press the "1" key.
> >
> >
> > It will navigate to "General setup"
> > instead of "Enable loadable module support".
>
>
> Hmm, this is a confusing issue.
> It will take you to the parent menu of the option, it should move you
> to the parent option and
> move the cursor to the option location. I will see if I can resolve
> this oversight.
>
> Thanks,
> Jesse Taube
>
> >
> >
> >
> >
> >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
Jesse T June 27, 2023, 3:08 a.m. UTC | #4
On Mon, Jun 26, 2023 at 8:38 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Sun, Jun 25, 2023 at 5:36 AM Jesse T <mr.bossman075@gmail.com> wrote:
> >
> > On Sat, Jun 24, 2023 at 4:11 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > On Sun, Jun 18, 2023 at 9:16 AM Jesse Taube <mr.bossman075@gmail.com> wrote:
> > > >
> > > > Menuconfig has a feature where you can "press the key in the (#) prefix
> > > > to jump directly to that location. You will be returned to the current
> > > > search results after exiting this new menu."
> > > >
> > > > This commit adds this feature to nconfig, with almost identical code.
> > > >
> > > > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> > >
> > >
> > > Setting the code duplication aside,
> >
> >
> > If it does function as expected will you accept the patch?
>
>
> Probably.
>
> The menuconfig code you copied is really ugly.

Yes, I saw and was surprised it was allowed, grandfathered in I guess.

> Actually, I was refactoring the search jump code a few
> months ago, but I did not get around to completing it.

This isn't on git.kernel.org, would you mind sharing it?
If you have any ideas on the best way to implement this,
I would love suggestions.

Thanks,
Jesse T

>
>
>
>
>
>
>
>
> > Should I mark the copied codes'  original location?
> >
> > > does this patch work correctly?
> > >
> > >
> > >
> > >
> > > $ make defconfig
> > > $ make nconfig
> > >
> > > Press F8
> > >
> > > Input "MODULES" in the search box.
> > >
> > > You will see:
> > >
> > > (1) -> Enable loadable module support
> > >
> > > Press the "1" key.
> > >
> > >
> > > It will navigate to "General setup"
> > > instead of "Enable loadable module support".
> >
> >
> > Hmm, this is a confusing issue.
> > It will take you to the parent menu of the option, it should move you
> > to the parent option and
> > move the cursor to the option location. I will see if I can resolve
> > this oversight.
> >
> > Thanks,
> > Jesse Taube
> >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > --
> > > Best Regards
> > > Masahiro Yamada
>
>
>
> --
> Best Regards
> Masahiro Yamada
Masahiro Yamada June 29, 2023, 3:50 p.m. UTC | #5
On Tue, Jun 27, 2023 at 12:08 PM Jesse T <mr.bossman075@gmail.com> wrote:
>
> On Mon, Jun 26, 2023 at 8:38 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Sun, Jun 25, 2023 at 5:36 AM Jesse T <mr.bossman075@gmail.com> wrote:
> > >
> > > On Sat, Jun 24, 2023 at 4:11 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > > >
> > > > On Sun, Jun 18, 2023 at 9:16 AM Jesse Taube <mr.bossman075@gmail.com> wrote:
> > > > >
> > > > > Menuconfig has a feature where you can "press the key in the (#) prefix
> > > > > to jump directly to that location. You will be returned to the current
> > > > > search results after exiting this new menu."
> > > > >
> > > > > This commit adds this feature to nconfig, with almost identical code.
> > > > >
> > > > > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> > > >
> > > >
> > > > Setting the code duplication aside,
> > >
> > >
> > > If it does function as expected will you accept the patch?
> >
> >
> > Probably.
> >
> > The menuconfig code you copied is really ugly.
>
> Yes, I saw and was surprised it was allowed, grandfathered in I guess.


Yes, you are grandfathered in.

Code refactoring is one of maintainer's jobs.




>
> > Actually, I was refactoring the search jump code a few
> > months ago, but I did not get around to completing it.
>
> This isn't on git.kernel.org, would you mind sharing it?
> If you have any ideas on the best way to implement this,
> I would love suggestions.


Digging into my local repository, it looks like
I had written up functional code already.

I just filled in the commit description.

I will post it.






> Thanks,
> Jesse T
>
> >
> >
> >
> >
> >
> >
> >
> >
> > > Should I mark the copied codes'  original location?
> > >
> > > > does this patch work correctly?
> > > >
> > > >
> > > >
> > > >
> > > > $ make defconfig
> > > > $ make nconfig
> > > >
> > > > Press F8
> > > >
> > > > Input "MODULES" in the search box.
> > > >
> > > > You will see:
> > > >
> > > > (1) -> Enable loadable module support
> > > >
> > > > Press the "1" key.
> > > >
> > > >
> > > > It will navigate to "General setup"
> > > > instead of "Enable loadable module support".
> > >
> > >
> > > Hmm, this is a confusing issue.
> > > It will take you to the parent menu of the option, it should move you
> > > to the parent option and
> > > move the cursor to the option location. I will see if I can resolve
> > > this oversight.
> > >
> > > Thanks,
> > > Jesse Taube
> > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > --
> > > > Best Regards
> > > > Masahiro Yamada
> >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
diff mbox series

Patch

diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 3ba8b1af390f..cbed2e15c579 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -234,6 +234,9 @@  search_help[] =
 "   is located.  A location followed by a [ = y] indicates that this is\n"
 "   a selectable menu item, and the current value is displayed inside\n"
 "   brackets.\n"
+"   Press the key in the (#) prefix to jump directly to that\n"
+"   location. You will be returned to the current search results\n"
+"   after exiting this new menu.\n"
 "o  The 'Selects:' line tells, what symbol will be automatically selected\n"
 "   if this symbol is selected (y or m).\n"
 "o  The 'Selected by' line tells what symbol has selected this symbol.\n"
@@ -685,6 +688,54 @@  static int do_exit(void)
 	return 0;
 }
 
+#define JUMP_NB			9
+
+struct update_text_data {
+	struct list_head *head;
+	struct menu **targets;
+	int *keys;
+};
+
+static size_t get_nth_line(char *buf, size_t n)
+{
+	size_t i = 0;
+
+	while (n > 0 && buf[i]) {
+		if (buf[i++] == '\n')
+			n--;
+	}
+	return i;
+}
+
+static void update_text(char *buf, size_t start, size_t end, void *_data)
+{
+	struct update_text_data *data = _data;
+	struct jump_key *pos;
+	int k = 0;
+
+	start = get_nth_line(buf, start);
+	end = get_nth_line(buf, end);
+
+	list_for_each_entry(pos, data->head, entries) {
+		if (pos->offset >= start && pos->offset < end) {
+			char header[4];
+
+			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, "   ");
+			}
+
+			memcpy(buf + pos->offset, header, sizeof(header) - 1);
+		}
+	}
+	data->keys[k] = 0;
+}
 
 static void search_conf(void)
 {
@@ -693,6 +744,7 @@  static void search_conf(void)
 	struct gstr title;
 	char *dialog_input;
 	int dres;
+	bool again = false;
 
 	title = str_new();
 	str_printf( &title, "Enter (sub)string or regexp to search for "
@@ -721,11 +773,30 @@  static void search_conf(void)
 		dialog_input += strlen(CONFIG_);
 
 	sym_arr = sym_re_search(dialog_input);
-	res = get_relations_str(sym_arr, NULL);
+
+	do {
+		LIST_HEAD(head);
+		struct menu *targets[10];
+		int keys[JUMP_NB + 1], i;
+		struct update_text_data data = {
+			.head = &head,
+			.targets = targets,
+			.keys = keys,
+		};
+
+		res = get_relations_str(sym_arr, &head);
+		dres = show_scroll_win_ext(main_window,
+				"Search Results", str_get(&res),
+				keys, &update_text, &data);
+		again = false;
+		for (i = 0; i < JUMP_NB && keys[i]; i++)
+			if (dres == keys[i]) {
+				conf(targets[i]->parent);
+				again = true;
+			}
+		str_free(&res);
+	} while (again);
 	free(sym_arr);
-	show_scroll_win(main_window,
-			"Search Results", str_get(&res));
-	str_free(&res);
 	str_free(&title);
 }
 
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index 9aedf40f1dc0..44ab0e43c626 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -497,10 +497,19 @@  void refresh_all_windows(WINDOW *main_window)
 	refresh();
 }
 
-/* layman's scrollable window... */
+/* Discards const qualifier, but char *text
+ * won't be modifed unless update_text_fnupdate_text is not NULL.
+ */
 void show_scroll_win(WINDOW *main_window,
 		const char *title,
 		const char *text)
+{
+	(void)show_scroll_win_ext(main_window, title, (char *)text, (int []) {0}, NULL, NULL);
+}
+
+/* layman's scrollable window... */
+int show_scroll_win_ext(WINDOW *main_window, const char *title, char *text,
+			 int *keys, update_text_fn update_text, void *data)
 {
 	int res;
 	int total_lines = get_line_no(text);
@@ -514,6 +523,7 @@  void show_scroll_win(WINDOW *main_window,
 	WINDOW *win;
 	WINDOW *pad;
 	PANEL *panel;
+	bool done = false;
 
 	getmaxyx(stdscr, lines, columns);
 
@@ -549,8 +559,11 @@  void show_scroll_win(WINDOW *main_window,
 	panel = new_panel(win);
 
 	/* handle scrolling */
-	do {
-
+	while (!done) {
+		if (update_text) {
+			update_text(text, start_y, start_y + text_lines, data);
+			fill_window(pad, text);
+		}
 		copywin(pad, win, start_y, start_x, 2, 2, text_lines,
 				text_cols, 0);
 		print_in_middle(win,
@@ -593,8 +606,15 @@  void show_scroll_win(WINDOW *main_window,
 		case 'l':
 			start_x++;
 			break;
+		default:
+			for (i = 0; keys[i]; i++) {
+				if (res == keys[i]) {
+					done = true;
+					break;
+				}
+			}
 		}
-		if (res == 10 || res == 27 || res == 'q' ||
+		if (res == 0 || res == 10 || res == 27 || res == 'q' ||
 			res == KEY_F(F_HELP) || res == KEY_F(F_BACK) ||
 			res == KEY_F(F_EXIT))
 			break;
@@ -606,9 +626,10 @@  void show_scroll_win(WINDOW *main_window,
 			start_x = 0;
 		if (start_x >= total_cols-text_cols)
 			start_x = total_cols-text_cols;
-	} while (res);
+	}
 
 	del_panel(panel);
 	delwin(win);
 	refresh_all_windows(main_window);
+	return res;
 }
diff --git a/scripts/kconfig/nconf.h b/scripts/kconfig/nconf.h
index 6f925bc74eb3..747b30a4cd0f 100644
--- a/scripts/kconfig/nconf.h
+++ b/scripts/kconfig/nconf.h
@@ -67,6 +67,9 @@  typedef enum {
 
 void set_colors(void);
 
+typedef void (*update_text_fn)(char *buf, size_t start, size_t end, void
+			       *_data);
+
 /* this changes the windows attributes !!! */
 void print_in_middle(WINDOW *win, int y, int width, const char *str, int attrs);
 int get_line_length(const char *line);
@@ -78,6 +81,8 @@  int dialog_inputbox(WINDOW *main_window,
 		const char *title, const char *prompt,
 		const char *init, char **resultp, int *result_len);
 void refresh_all_windows(WINDOW *main_window);
+int show_scroll_win_ext(WINDOW *main_window, const char *title, char *text,
+			 int *keys, update_text_fn update_text, void *data);
 void show_scroll_win(WINDOW *main_window,
 		const char *title,
 		const char *text);