| Message ID | 20211213100043.45645-2-arielmarcovitch@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kconfig: Improve comment blocks in the .config file | expand |
On Mon, Dec 13, 2021 at 7:01 PM Ariel Marcovitch <arielmarcovitch@gmail.com> wrote: > > Until now, menuconfigs were considered configs because they had non-zero > sym attribute. This meant that instead of having the nice menu comment > block in the .config output file, they were merely shown as single > configs. > > For example: > ```Kconfig > menu "Foo" > endmenu > > menuconfig BAR > bool "Bar" > > config OTHER > bool "Other" > depends on BAR > ``` > > Will be shown as: > ```.config > # > # Foo > # > # end of Foo I am OK with this patch. Just a nit. As far as I tested your sample code (without applying this patch), I did not see the line "# end of Foo". The line "# end of ..." is printed when the last child gets back to its parent, but the "Foo" menu has no child menu here. This is out of scope of this patch, but can you update the commit log so it matches the current behavior? (or add one config into the "Foo" menu) > > CONFIG_BAR=y > CONFIG_OTHER=y > ``` > > Instead of using the sym attribute to decide whether or not to print the > menu block comment, check menu->prompt->type explicitly (after checking > that menu_is_visible(menu) which means menu->prompt is not none). The > only prompt types we actually show as menus are P_MENU and P_COMMENT. At > the end of the menu we need to show the end of block only for P_MENU > (although P_COMMENT prompts will not get to this flow because they don't > have children). > > Signed-off-by: Ariel Marcovitch <arielmarcovitch@gmail.com> > --- > scripts/kconfig/confdata.c | 28 +++++++++++++++++----------- > 1 file changed, 17 insertions(+), 11 deletions(-) > > diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c > index 42bc56ee238c..9f2c22f46ee0 100644 > --- a/scripts/kconfig/confdata.c > +++ b/scripts/kconfig/confdata.c > @@ -874,16 +874,21 @@ int conf_write(const char *name) > menu = rootmenu.list; > while (menu) { > sym = menu->sym; > - if (!sym) { > - if (!menu_is_visible(menu)) > - goto next; > - str = menu_get_prompt(menu); > - fprintf(out, "\n" > - "#\n" > - "# %s\n" > - "#\n", str); > - need_newline = false; > - } else if (!(sym->flags & SYMBOL_CHOICE) && > + > + if (menu_is_visible(menu)) { > + enum prop_type type = menu->prompt->type; > + > + if (type == P_MENU || type == P_COMMENT) { > + str = menu_get_prompt(menu); > + fprintf(out, "\n" > + "#\n" > + "# %s\n" > + "#\n", str); > + need_newline = false; > + } > + } > + > + if (sym && !(sym->flags & SYMBOL_CHOICE) && > !(sym->flags & SYMBOL_WRITTEN)) { > sym_calc_value(sym); > if (!(sym->flags & SYMBOL_WRITE)) > @@ -904,7 +909,8 @@ int conf_write(const char *name) > if (menu->next) > menu = menu->next; > else while ((menu = menu->parent)) { > - if (!menu->sym && menu_is_visible(menu) && > + if (menu_is_visible(menu) && > + menu->prompt->type == P_MENU && > menu != &rootmenu) { > str = menu_get_prompt(menu); > fprintf(out, "# end of %s\n", str); > -- > 2.25.1 >
Hello! On 18/01/2022 20:20, Masahiro Yamada wrote: > On Mon, Dec 13, 2021 at 7:01 PM Ariel Marcovitch > <arielmarcovitch@gmail.com> wrote: >> Until now, menuconfigs were considered configs because they had non-zero >> sym attribute. This meant that instead of having the nice menu comment >> block in the .config output file, they were merely shown as single >> configs. >> >> For example: >> ```Kconfig >> menu "Foo" >> endmenu >> >> menuconfig BAR >> bool "Bar" >> >> config OTHER >> bool "Other" >> depends on BAR >> ``` >> >> Will be shown as: >> ```.config >> # >> # Foo >> # >> # end of Foo > > I am OK with this patch. > > Just a nit. > > As far as I tested your sample code (without applying this patch), > I did not see the line "# end of Foo". > > The line "# end of ..." is printed when the last child gets back to > its parent, but the "Foo" menu has no child menu here. > > This is out of scope of this patch, but can you update the > commit log so it matches the current behavior? I saw you added a patch to change that, so now the code sample here is less of a lie :) I learned my message of never adding code samples to commit messages without testing these as well :) So is it ready now to be applied on top of your change? > (or add one config into the "Foo" menu) > > > > > > > >> CONFIG_BAR=y >> CONFIG_OTHER=y >> ``` >> >> Instead of using the sym attribute to decide whether or not to print the >> menu block comment, check menu->prompt->type explicitly (after checking >> that menu_is_visible(menu) which means menu->prompt is not none). The >> only prompt types we actually show as menus are P_MENU and P_COMMENT. At >> the end of the menu we need to show the end of block only for P_MENU >> (although P_COMMENT prompts will not get to this flow because they don't >> have children). >> >> Signed-off-by: Ariel Marcovitch <arielmarcovitch@gmail.com> >> --- >> scripts/kconfig/confdata.c | 28 +++++++++++++++++----------- >> 1 file changed, 17 insertions(+), 11 deletions(-) >> >> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c >> index 42bc56ee238c..9f2c22f46ee0 100644 >> --- a/scripts/kconfig/confdata.c >> +++ b/scripts/kconfig/confdata.c >> @@ -874,16 +874,21 @@ int conf_write(const char *name) >> menu = rootmenu.list; >> while (menu) { >> sym = menu->sym; >> - if (!sym) { >> - if (!menu_is_visible(menu)) >> - goto next; >> - str = menu_get_prompt(menu); >> - fprintf(out, "\n" >> - "#\n" >> - "# %s\n" >> - "#\n", str); >> - need_newline = false; >> - } else if (!(sym->flags & SYMBOL_CHOICE) && >> + >> + if (menu_is_visible(menu)) { >> + enum prop_type type = menu->prompt->type; >> + >> + if (type == P_MENU || type == P_COMMENT) { >> + str = menu_get_prompt(menu); >> + fprintf(out, "\n" >> + "#\n" >> + "# %s\n" >> + "#\n", str); >> + need_newline = false; >> + } >> + } >> + >> + if (sym && !(sym->flags & SYMBOL_CHOICE) && >> !(sym->flags & SYMBOL_WRITTEN)) { >> sym_calc_value(sym); >> if (!(sym->flags & SYMBOL_WRITE)) >> @@ -904,7 +909,8 @@ int conf_write(const char *name) >> if (menu->next) >> menu = menu->next; >> else while ((menu = menu->parent)) { >> - if (!menu->sym && menu_is_visible(menu) && >> + if (menu_is_visible(menu) && >> + menu->prompt->type == P_MENU && >> menu != &rootmenu) { >> str = menu_get_prompt(menu); >> fprintf(out, "# end of %s\n", str); >> -- >> 2.25.1 >>
On Sat, Feb 19, 2022 at 3:39 AM Ariel Marcovitch <arielmarcovitch@gmail.com> wrote: > > Hello! > > On 18/01/2022 20:20, Masahiro Yamada wrote: > > On Mon, Dec 13, 2021 at 7:01 PM Ariel Marcovitch > > <arielmarcovitch@gmail.com> wrote: > >> Until now, menuconfigs were considered configs because they had non-zero > >> sym attribute. This meant that instead of having the nice menu comment > >> block in the .config output file, they were merely shown as single > >> configs. > >> > >> For example: > >> ```Kconfig > >> menu "Foo" > >> endmenu > >> > >> menuconfig BAR > >> bool "Bar" > >> > >> config OTHER > >> bool "Other" > >> depends on BAR > >> ``` > >> > >> Will be shown as: > >> ```.config > >> # > >> # Foo > >> # > >> # end of Foo > > > > I am OK with this patch. > > > > Just a nit. > > > > As far as I tested your sample code (without applying this patch), > > I did not see the line "# end of Foo". > > > > The line "# end of ..." is printed when the last child gets back to > > its parent, but the "Foo" menu has no child menu here. > > > > This is out of scope of this patch, but can you update the > > commit log so it matches the current behavior? > > I saw you added a patch to change that, so now the code sample here is > less of a lie :) > > I learned my message of never adding code samples to commit messages > without testing these as well :) > > So is it ready now to be applied on top of your change? Yes, v2 please. > > (or add one config into the "Foo" menu) > > > > > > > > > > > > > > > >> CONFIG_BAR=y > >> CONFIG_OTHER=y > >> ``` > >> > >> Instead of using the sym attribute to decide whether or not to print the > >> menu block comment, check menu->prompt->type explicitly (after checking > >> that menu_is_visible(menu) which means menu->prompt is not none). The > >> only prompt types we actually show as menus are P_MENU and P_COMMENT. At > >> the end of the menu we need to show the end of block only for P_MENU > >> (although P_COMMENT prompts will not get to this flow because they don't > >> have children). > >> > >> Signed-off-by: Ariel Marcovitch <arielmarcovitch@gmail.com> > >> --- > >> scripts/kconfig/confdata.c | 28 +++++++++++++++++----------- > >> 1 file changed, 17 insertions(+), 11 deletions(-) > >> > >> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c > >> index 42bc56ee238c..9f2c22f46ee0 100644 > >> --- a/scripts/kconfig/confdata.c > >> +++ b/scripts/kconfig/confdata.c > >> @@ -874,16 +874,21 @@ int conf_write(const char *name) > >> menu = rootmenu.list; > >> while (menu) { > >> sym = menu->sym; > >> - if (!sym) { > >> - if (!menu_is_visible(menu)) > >> - goto next; > >> - str = menu_get_prompt(menu); > >> - fprintf(out, "\n" > >> - "#\n" > >> - "# %s\n" > >> - "#\n", str); > >> - need_newline = false; > >> - } else if (!(sym->flags & SYMBOL_CHOICE) && > >> + > >> + if (menu_is_visible(menu)) { > >> + enum prop_type type = menu->prompt->type; > >> + > >> + if (type == P_MENU || type == P_COMMENT) { > >> + str = menu_get_prompt(menu); > >> + fprintf(out, "\n" > >> + "#\n" > >> + "# %s\n" > >> + "#\n", str); > >> + need_newline = false; > >> + } > >> + } > >> + > >> + if (sym && !(sym->flags & SYMBOL_CHOICE) && > >> !(sym->flags & SYMBOL_WRITTEN)) { > >> sym_calc_value(sym); > >> if (!(sym->flags & SYMBOL_WRITE)) > >> @@ -904,7 +909,8 @@ int conf_write(const char *name) > >> if (menu->next) > >> menu = menu->next; > >> else while ((menu = menu->parent)) { > >> - if (!menu->sym && menu_is_visible(menu) && > >> + if (menu_is_visible(menu) && > >> + menu->prompt->type == P_MENU && > >> menu != &rootmenu) { > >> str = menu_get_prompt(menu); > >> fprintf(out, "# end of %s\n", str); > >> -- > >> 2.25.1 > >>
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c index 42bc56ee238c..9f2c22f46ee0 100644 --- a/scripts/kconfig/confdata.c +++ b/scripts/kconfig/confdata.c @@ -874,16 +874,21 @@ int conf_write(const char *name) menu = rootmenu.list; while (menu) { sym = menu->sym; - if (!sym) { - if (!menu_is_visible(menu)) - goto next; - str = menu_get_prompt(menu); - fprintf(out, "\n" - "#\n" - "# %s\n" - "#\n", str); - need_newline = false; - } else if (!(sym->flags & SYMBOL_CHOICE) && + + if (menu_is_visible(menu)) { + enum prop_type type = menu->prompt->type; + + if (type == P_MENU || type == P_COMMENT) { + str = menu_get_prompt(menu); + fprintf(out, "\n" + "#\n" + "# %s\n" + "#\n", str); + need_newline = false; + } + } + + if (sym && !(sym->flags & SYMBOL_CHOICE) && !(sym->flags & SYMBOL_WRITTEN)) { sym_calc_value(sym); if (!(sym->flags & SYMBOL_WRITE)) @@ -904,7 +909,8 @@ int conf_write(const char *name) if (menu->next) menu = menu->next; else while ((menu = menu->parent)) { - if (!menu->sym && menu_is_visible(menu) && + if (menu_is_visible(menu) && + menu->prompt->type == P_MENU && menu != &rootmenu) { str = menu_get_prompt(menu); fprintf(out, "# end of %s\n", str);
Until now, menuconfigs were considered configs because they had non-zero sym attribute. This meant that instead of having the nice menu comment block in the .config output file, they were merely shown as single configs. For example: ```Kconfig menu "Foo" endmenu menuconfig BAR bool "Bar" config OTHER bool "Other" depends on BAR ``` Will be shown as: ```.config # # Foo # # end of Foo CONFIG_BAR=y CONFIG_OTHER=y ``` Instead of using the sym attribute to decide whether or not to print the menu block comment, check menu->prompt->type explicitly (after checking that menu_is_visible(menu) which means menu->prompt is not none). The only prompt types we actually show as menus are P_MENU and P_COMMENT. At the end of the menu we need to show the end of block only for P_MENU (although P_COMMENT prompts will not get to this flow because they don't have children). Signed-off-by: Ariel Marcovitch <arielmarcovitch@gmail.com> --- scripts/kconfig/confdata.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-)