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

Message ID 20230629160351.2996541-2-masahiroy@kernel.org
State New
Headers
Series [1/2] kconfig: menuconfig: simplify global jump key assignment |

Commit Message

Masahiro Yamada June 29, 2023, 4:03 p.m. UTC
  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(-)
  

Comments

Jesse T July 1, 2023, 4:08 a.m. UTC | #1
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
  
Randy Dunlap July 1, 2023, 4:23 a.m. UTC | #2
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.
  
Jesse T July 1, 2023, 4:38 a.m. UTC | #3
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
  
Randy Dunlap July 1, 2023, 4:43 a.m. UTC | #4
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?
  
Masahiro Yamada July 2, 2023, 3:27 p.m. UTC | #5
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
  

Patch

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);
 	}