Re: [PATCH 2/2] kconfig: Make comments look different than menus in .config

[Ariel Marcovitch]

On 18/01/2022 20:25, Masahiro Yamada wrote:
> On Mon, Dec 13, 2021 at 7:01 PM Ariel Marcovitch
> <arielmarcovitch@xxxxxxxxx> wrote:
> > Currently, the same code that handles menus in the write to .config
> > handles comments as well. That's why comments look exactly like menus in
> > the .config except for the 'end of menu' comments that appear only for
> > menus. This makes sense because sometimes comments are used as sort of
> > submenus. However for the other cases, it looks kinda weird because one
> > might attempt to look for the 'end of menu' for comments as well and be
> > very confused.
> >
> > Make comments look different than menus. For the following:
> > ```kconfig
> > menu "Stuff"
> >
> > config FOO
> >          def_bool y
> >
> > comment "Some comment"
> >
> > config BAR
> >          def_bool n
> >
> > endmenu
> > ```
> >
> > The .config will look like this:
> > ```
> >   #
> >   # Stuff
> >   #
> >   CONFIG_FOO=y
> >
> >   ### Some comment
> >   # CONFIG_BAR is not defined
> >   # end of Stuff
> >
> > ```
> >
> > Signed-off-by: Ariel Marcovitch <arielmarcovitch@xxxxxxxxx>
> > ---
> >   scripts/kconfig/confdata.c | 14 ++++++++++----
> >   1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> > index 9f2c22f46ee0..d3ec1ad67d92 100644
> > --- a/scripts/kconfig/confdata.c
> > +++ b/scripts/kconfig/confdata.c
> > @@ -880,10 +880,16 @@ int conf_write(const char *name)
> >
> >                          if (type == P_MENU || type == P_COMMENT) {
> >                                  str = menu_get_prompt(menu);
> > -                               fprintf(out, "\n"
> > -                                       "#\n"
> > -                                       "# %s\n"
> > -                                       "#\n", str);
> > +
> > +                               if (type == P_MENU)
> > +                                       fprintf(out, "\n"
> > +                                               "#\n"
> > +                                               "# %s\n"
> > +                                               "#\n", str);
> > +                               else
> > +                                       fprintf(out, "\n"
> > +                                               "### %s\n", str);
> > +
> >                                  need_newline = false;
> >                          }
> >                  }
> > --
> > 2.25.1
>
> Since "# CONFIG... is not set" looks like a comment,
> I am not sure if this improves the visibility.

I agree that adding another '#' signs to the real comments doesn't solve 
the real
problem here, being that kconfig uses comments to save actual information

I guess this is for being able to check for a config in shell script 
with [[ -n $CONFIG_FOO ]]?

Although if that's the case, leaving the config empty has the same 
effect, no? And then
we can add a comment to the end of the definition stating that the 
config is unset.
Something like this:

CONFIG_FOO=y
CONFIG_BAR= # is not set

It may break scripts doing something like this:

: ${CONFIG_FOO=?Config FOO must be defined}

But they can be changed to use ':?' instead (which checks for non-zero 
length string
rather than whether the variable is defined or not)

Actually, now that I think of it, it might even be an improvement for 
scripts to be able to tell whether a config isn't defined or whether it 
has an 'n' value

Anyway, I'm absolutely fine with delaying this patch until we find a 
solution

> I will not pick up this until I find out what a really good format is.
Thanks!