* [PATCH] kconfig: Create links to main menu items in search
@ 2021-09-01 17:53 Ariel Marcovitch
2021-09-10 2:14 ` Masahiro Yamada
0 siblings, 1 reply; 4+ messages in thread
From: Ariel Marcovitch @ 2021-09-01 17:53 UTC (permalink / raw)
To: masahiroy; +Cc: arielmarcovitch, linux-kernel, linux-kbuild
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>
---
| 40 ++++++++++++++++++++++++++--------------
1 file changed, 26 insertions(+), 14 deletions(-)
--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;
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;
+ }
+
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";
+ else
+ prompt = menu_get_prompt(menu);
+
+ 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] kconfig: Create links to main menu items in search
2021-09-01 17:53 [PATCH] kconfig: Create links to main menu items in search Ariel Marcovitch
@ 2021-09-10 2:14 ` Masahiro Yamada
2021-09-11 18:18 ` Ariel Marcovitch
0 siblings, 1 reply; 4+ messages in thread
From: Masahiro Yamada @ 2021-09-10 2:14 UTC (permalink / raw)
To: Ariel Marcovitch; +Cc: Linux Kernel Mailing List, Linux Kbuild mailing list
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) {
> 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?
> + 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);
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] kconfig: Create links to main menu items in search
2021-09-10 2:14 ` Masahiro Yamada
@ 2021-09-11 18:18 ` Ariel Marcovitch
2021-09-12 11:37 ` Masahiro Yamada
0 siblings, 1 reply; 4+ messages in thread
From: Ariel Marcovitch @ 2021-09-11 18:18 UTC (permalink / raw)
To: Masahiro Yamada; +Cc: Linux Kernel Mailing List, Linux Kbuild mailing list
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] kconfig: Create links to main menu items in search
2021-09-11 18:18 ` Ariel Marcovitch
@ 2021-09-12 11:37 ` Masahiro Yamada
0 siblings, 0 replies; 4+ messages in thread
From: Masahiro Yamada @ 2021-09-12 11:37 UTC (permalink / raw)
To: Ariel Marcovitch; +Cc: Linux Kernel Mailing List, Linux Kbuild mailing list
On Sun, Sep 12, 2021 at 3:18 AM Ariel Marcovitch
<arielmarcovitch@gmail.com> wrote:
>
> 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?
I think so.
The result is more consistent.
> >
> >
> >
> >
> >
> >
> >
> >>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.
I am fine with it.
> Also, the results look a bit weird... maybe we should use a different
> marker for rootmenu, like '--' or something?
How does it look weird?
> >
> >
> >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
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-09-12 11:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-01 17:53 [PATCH] kconfig: Create links to main menu items in search Ariel Marcovitch
2021-09-10 2:14 ` Masahiro Yamada
2021-09-11 18:18 ` Ariel Marcovitch
2021-09-12 11:37 ` Masahiro Yamada
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.