Re: [PATCH] kconfig: Create links to main menu items in search

[Ariel Marcovitch <arielmarcovitch@gmail.com>]

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