Linux kbuild/kconfig development
 help / color / mirror / Atom feed
From: Ariel Marcovitch <arielmarcovitch@gmail.com>
To: Masahiro Yamada <masahiroy@kernel.org>
Cc: Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] kconfig: Show menuconfigs as menus in the .config file
Date: Fri, 18 Feb 2022 20:39:24 +0200	[thread overview]
Message-ID: <26d74eaa-5c6a-4103-cf77-1356173a3978@gmail.com> (raw)
In-Reply-To: <CAK7LNAS+Df_V-B9Qy_39hgUZF1b6UeiHQ5m-25JekiVYSQ67dQ@mail.gmail.com>

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
>>

  reply	other threads:[~2022-02-18 18:39 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-13 10:00 [PATCH 0/2] kconfig: Improve comment blocks in the .config file Ariel Marcovitch
2021-12-13 10:00 ` [PATCH 1/2] kconfig: Show menuconfigs as menus " Ariel Marcovitch
2022-01-18 18:20   ` Masahiro Yamada
2022-02-18 18:39     ` Ariel Marcovitch [this message]
2022-02-20  2:23       ` Masahiro Yamada
2021-12-13 10:00 ` [PATCH 2/2] kconfig: Make comments look different than menus in .config Ariel Marcovitch
2022-01-18 18:25   ` Masahiro Yamada
2022-02-18 18:55     ` Ariel Marcovitch
2022-02-20  4:17       ` Masahiro Yamada
2021-12-13 11:47 ` [PATCH 0/2] kconfig: Improve comment blocks in the .config file Boris Kolpackov
2021-12-14  8:27   ` Ariel Marcovitch
2022-01-15 13:58 ` Ariel Marcovitch
2022-01-15 13:58 ` Ariel Marcovitch

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=26d74eaa-5c6a-4103-cf77-1356173a3978@gmail.com \
    --to=arielmarcovitch@gmail.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox