Messages in this thread | | | Subject | Re: [PATCH] kconfig: Create links to main menu items in search | From | Ariel Marcovitch <> | Date | Sat, 11 Sep 2021 21:18:24 +0300 |
| |
On 10/09/2021 5:14, Masahiro Yamada wrote: >On Thu, Sep 2, 2021 at 2:53 AM Ariel Marcovitch ><arielmarcovitch@gmail.com> wrote: >> >>When one searches for a main menu item, links aren't created for it like >>with the rest of the symbols. >> >>This happens because we trace the item until we get to the rootmenu, but >>we don't include it in the path of the item. The rationale was probably >>that we don't want to show the main menu in the path of all items, >>because it is redundant. >> >>However, when an item has only the rootmenu in its path it should be >>included, because this way the user can jump to its location. >> >>In case the item is a direct child of the rootmenu, show it in the >>'Location:' section as 'Main Menu'. >> >>This makes the 'if (i > 0)' superfluous because each item with prompt >>will have at least one menu in its path. >> >>Signed-off-by: Ariel Marcovitch <arielmarcovitch@gmail.com> >>--- >>scripts/kconfig/menu.c | 40 ++++++++++++++++++++++++++-------------- >>1 file changed, 26 insertions(+), 14 deletions(-) >> >>diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c >>index 606ba8a63c24..8d7e3b07bf93 100644 >>--- a/scripts/kconfig/menu.c >>+++ b/scripts/kconfig/menu.c >>@@ -712,6 +712,7 @@ static void get_prompt_str(struct gstr *r, struct property *prop, >>int i, j; >>struct menu *submenu[8], *menu, *location = NULL; >>struct jump_key *jump = NULL; >>+const char *prompt = NULL; > > >Can you move this to the for-loop ? > >The initializer is unneeded. > > >> >>str_printf(r, " Prompt: %s\n", prop->text); >> >>@@ -735,6 +736,13 @@ static void get_prompt_str(struct gstr *r, struct property *prop, >>if (location == NULL && accessible) >>location = menu; >>} >>+ >>+/* If we have only the root menu, show it */ >>+if (i == 0) { >>+location = &rootmenu; >>+submenu[i++] = location; >>+} > > >Instead of handling this as a special case, >can we include the rootmenu all the time? > >We can change the for-loop condition to: > >for (i = 0; menu && i < 8; menu = menu->parent) { Of course. However, it means search entries will get a bit larger. I guess it is worth it? > > > > > > > >>if (head && location) { >>jump = xmalloc(sizeof(struct jump_key)); >> >>@@ -758,21 +766,25 @@ static void get_prompt_str(struct gstr *r, struct property *prop, >>list_add_tail(&jump->entries, head); >>} >> >>-if (i > 0) { >>-str_printf(r, " Location:\n"); >>-for (j = 4; --i >= 0; j += 2) { >>-menu = submenu[i]; >>-if (jump && menu == location) >>-jump->offset = strlen(r->s); >>-str_printf(r, "%*c-> %s", j, ' ', >>-menu_get_prompt(menu)); >>-if (menu->sym) { >>-str_printf(r, " (%s [=%s])", menu->sym->name ? >>-menu->sym->name : "<choice>", >>-sym_get_string_value(menu->sym)); >>-} >>-str_append(r, "\n"); >>+str_printf(r, " Location:\n"); >>+for (j = 4; --i >= 0; j += 2) { >>+menu = submenu[i]; >>+if (jump && menu == location) >>+jump->offset = strlen(r->s); >>+ >>+/* The real rootmenu prompt is ugly */ >>+if (menu == &rootmenu) >>+prompt = "Main Menu"; > >Can you use "Main menu" for the consistency >with scripts/kconfig/parser.y line 501? > Seems reasonable. Just to clarify, the prompt there is not relevant for linux's Kconfig right? > >>+else >>+prompt = menu_get_prompt(menu); > > >I think it is better to omit '->' for the rootmenu. > > >if (menu == &rootmenu) { >prompt = "Main menu"; >marker = ""; >} else { >prompt = menu_get_prompt(menu); >marker = "->"; >} > >str_printf(r, "%*c%s %s", j, ' ', marker, prompt); > Perhaps it will be better to split to separate printfs? I think it will be cleaner. It will make the extra vars unneeded as well. Also, the results look a bit weird... maybe we should use a different marker for rootmenu, like '--' or something? > > >Maybe, this will make the help look cleaner. > > > > > >>+str_printf(r, "%*c-> %s", j, ' ', prompt); >>+if (menu->sym) { >>+str_printf(r, " (%s [=%s])", menu->sym->name ? >>+menu->sym->name : "<choice>", >>+sym_get_string_value(menu->sym)); >>} >>+str_append(r, "\n"); >>} >>} >> >> >>base-commit: 087e856cfb76e9eef9a3a6e000854794f3c36e24 >>-- >>2.25.1 >> > > >-- >Best Regards > >Masahiro Yamada Thanks for the comments :) Ariel Marcovitch
| |