diff mbox series

[v1] kconfig: nconf: Keep position after jump search

Message ID 20230806032026.1718752-1-Mr.Bossman075@gmail.com (mailing list archive)
State Handled Elsewhere
Headers show
Series [v1] kconfig: nconf: Keep position after jump search | expand

Commit Message

Jesse Taube Aug. 6, 2023, 3:20 a.m. UTC
In this Menuconfig, pressing the key in the (#) prefix will jump
directly to that location. You will be returned to the current search
results after exiting this new menu.

In nconfig, exiting always returns to the top of the search output, not
to where the (#) was displayed on the search output screen.

This patch fixes that by saving the current position in the search.

Reported-by: Randy Dunlap <rdunlap@infradead.org>
Link: https://lore.kernel.org/r/20230805034445.2508362-1-Mr.Bossman075@gmail.com/
Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
---
 scripts/kconfig/nconf.c     |  3 ++-
 scripts/kconfig/nconf.gui.c | 12 +++++++++++-
 scripts/kconfig/nconf.h     |  1 +
 3 files changed, 14 insertions(+), 2 deletions(-)

Comments

Randy Dunlap Aug. 6, 2023, 3:30 p.m. UTC | #1
Hi Jesse,

On 8/5/23 20:20, Jesse Taube wrote:
> In this Menuconfig, pressing the key in the (#) prefix will jump
> directly to that location. You will be returned to the current search
> results after exiting this new menu.
> 
> In nconfig, exiting always returns to the top of the search output, not
> to where the (#) was displayed on the search output screen.
> 
> This patch fixes that by saving the current position in the search.
> 

This patch fixes the reported problem. Thanks, Jesse.

> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Link: https://lore.kernel.org/r/20230805034445.2508362-1-Mr.Bossman075@gmail.com/
> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>

Tested-by: Randy Dunlap <rdunlap@infradead.org>

Now I have another issue. :(

Here is my test case: x86_64 defconfig.

SymSearch (F8) for MSR.
Page Down to #3. Select 3.
X86_MSR is about 7 lines below the highlighted line in the menu,
which is confusing and sometimes it's not obvious what the correct
line for the symbol is.

In menuconfig, the highlighted line is precisely on X86_MSR.

Can nconfig be more precise about which menu line it highlights?

Thanks for your help.

> ---
>  scripts/kconfig/nconf.c     |  3 ++-
>  scripts/kconfig/nconf.gui.c | 12 +++++++++++-
>  scripts/kconfig/nconf.h     |  1 +
>  3 files changed, 14 insertions(+), 2 deletions(-)
Jesse Taube Aug. 6, 2023, 10:17 p.m. UTC | #2
On Sun, Aug 6, 2023 at 11:30 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> Hi Jesse,
>
> On 8/5/23 20:20, Jesse Taube wrote:
> > In this Menuconfig, pressing the key in the (#) prefix will jump
> > directly to that location. You will be returned to the current search
> > results after exiting this new menu.
> >
> > In nconfig, exiting always returns to the top of the search output, not
> > to where the (#) was displayed on the search output screen.
> >
> > This patch fixes that by saving the current position in the search.
> >
>
> This patch fixes the reported problem. Thanks, Jesse.
>
> > Reported-by: Randy Dunlap <rdunlap@infradead.org>
> > Link: https://lore.kernel.org/r/20230805034445.2508362-1-Mr.Bossman075@gmail.com/
> > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
>
> Tested-by: Randy Dunlap <rdunlap@infradead.org>
>
> Now I have another issue. :(
>
> Here is my test case: x86_64 defconfig.
>
> SymSearch (F8) for MSR.
> Page Down to #3. Select 3.
> X86_MSR is about 7 lines below the highlighted line in the menu,
> which is confusing and sometimes it's not obvious what the correct
> line for the symbol is.
>
> In menuconfig, the highlighted line is precisely on X86_MSR.

Oh jeez, how did I miss this?
I will fix this asap as this seems to be a big issue as it can easily
cause confusion.
Weirdly all the test cases I used were fine, but I found a few more like this.

Thanks,
Jesse Taube
>
> Can nconfig be more precise about which menu line it highlights?
>
> Thanks for your help.
>
> > ---
> >  scripts/kconfig/nconf.c     |  3 ++-
> >  scripts/kconfig/nconf.gui.c | 12 +++++++++++-
> >  scripts/kconfig/nconf.h     |  1 +
> >  3 files changed, 14 insertions(+), 2 deletions(-)
>
>
> --
> ~Randy
Masahiro Yamada Aug. 7, 2023, 2:37 a.m. UTC | #3
On Mon, Aug 7, 2023 at 7:17 AM Jesse T <mr.bossman075@gmail.com> wrote:
>
> On Sun, Aug 6, 2023 at 11:30 AM Randy Dunlap <rdunlap@infradead.org> wrote:
> >
> > Hi Jesse,
> >
> > On 8/5/23 20:20, Jesse Taube wrote:
> > > In this Menuconfig, pressing the key in the (#) prefix will jump
> > > directly to that location. You will be returned to the current search
> > > results after exiting this new menu.
> > >
> > > In nconfig, exiting always returns to the top of the search output, not
> > > to where the (#) was displayed on the search output screen.
> > >
> > > This patch fixes that by saving the current position in the search.
> > >
> >
> > This patch fixes the reported problem. Thanks, Jesse.
> >
> > > Reported-by: Randy Dunlap <rdunlap@infradead.org>
> > > Link: https://lore.kernel.org/r/20230805034445.2508362-1-Mr.Bossman075@gmail.com/
> > > Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> >
> > Tested-by: Randy Dunlap <rdunlap@infradead.org>
> >
> > Now I have another issue. :(
> >
> > Here is my test case: x86_64 defconfig.
> >
> > SymSearch (F8) for MSR.
> > Page Down to #3. Select 3.
> > X86_MSR is about 7 lines below the highlighted line in the menu,
> > which is confusing and sometimes it's not obvious what the correct
> > line for the symbol is.
> >
> > In menuconfig, the highlighted line is precisely on X86_MSR.
>
> Oh jeez, how did I miss this?
> I will fix this asap as this seems to be a big issue as it can easily
> cause confusion.
> Weirdly all the test cases I used were fine, but I found a few more like this.


As it turns out, your initial nconf search is not mature enough.

I will drop it from my tree.
diff mbox series

Patch

diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 172dc8fdd3ef..536332286c2f 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -749,7 +749,7 @@  static void search_conf(void)
 	struct gstr res;
 	struct gstr title;
 	char *dialog_input;
-	int dres;
+	int dres, vscroll = 0, hscroll = 0;
 	bool again;
 
 	title = str_new();
@@ -790,6 +790,7 @@  static void search_conf(void)
 		res = get_relations_str(sym_arr, &head);
 		dres = show_scroll_win_ext(main_window,
 				"Search Results", str_get(&res),
+				&vscroll, &hscroll,
 				handle_search_keys, &data);
 		again = false;
 		if (dres >= '1' && dres <= '9') {
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index bf015895053c..5c54d6fec460 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -501,11 +501,12 @@  void show_scroll_win(WINDOW *main_window,
 		const char *title,
 		const char *text)
 {
-	(void)show_scroll_win_ext(main_window, title, (char *)text, NULL, NULL);
+	(void)show_scroll_win_ext(main_window, title, (char *)text, NULL, NULL, NULL, NULL);
 }
 
 /* layman's scrollable window... */
 int show_scroll_win_ext(WINDOW *main_window, const char *title, char *text,
+			int *vscroll, int *hscroll,
 			extra_key_cb_fn extra_key_cb, void *data)
 {
 	int res;
@@ -522,6 +523,11 @@  int show_scroll_win_ext(WINDOW *main_window, const char *title, char *text,
 	PANEL *panel;
 	bool done = false;
 
+	if (hscroll)
+		start_x = *hscroll;
+	if (vscroll)
+		start_y = *vscroll;
+
 	getmaxyx(stdscr, lines, columns);
 
 	/* find the widest line of msg: */
@@ -624,6 +630,10 @@  int show_scroll_win_ext(WINDOW *main_window, const char *title, char *text,
 			start_x = total_cols-text_cols;
 	}
 
+	if (hscroll)
+		*hscroll = start_x;
+	if (vscroll)
+		*vscroll = start_y;
 	del_panel(panel);
 	delwin(win);
 	refresh_all_windows(main_window);
diff --git a/scripts/kconfig/nconf.h b/scripts/kconfig/nconf.h
index 912f668c5772..ab836d582664 100644
--- a/scripts/kconfig/nconf.h
+++ b/scripts/kconfig/nconf.h
@@ -81,6 +81,7 @@  int dialog_inputbox(WINDOW *main_window,
 		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 *vscroll, int *hscroll,
 			extra_key_cb_fn extra_key_cb, void *data);
 void show_scroll_win(WINDOW *main_window,
 		const char *title,