[PATCH 0/2] kconfig: 'if' and automatic submenu creation documentation

[PATCH 0/2] kconfig: 'if' and automatic submenu creation documentation

[Ulf Magnusson]

These together should make the automatic submenu logic a lot clearer.

Ulf Magnusson (2):
  kconfig: Document 'if' flattening logic
  kconfig: Improve auto. menu documentation accuracy

 scripts/kconfig/menu.c | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

-- 
2.14.1

[PATCH 1/2] kconfig: Document 'if' flattening logic

[Ulf Magnusson]

It is not obvious that this refers to an 'if', making the code pretty
cryptic:

	if (menu->list && (!menu->prompt || !menu->prompt->text)) {

Kconfig keeps the 'if' menu nodes even after flattening. Reflect that in
the example to be accurate.

No functional changes. Only comments added.

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
---
 scripts/kconfig/menu.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 372eb5d9fef3..1f7bcceacde3 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -524,6 +524,30 @@ void menu_finalize(struct menu *parent)
 			*ep = expr_alloc_one(E_LIST, NULL);
 			(*ep)->right.sym = menu->sym;
 		}
+
+		/*
+		 * Flatten 'if' blocks, which do not specify a submenu and only
+		 * add dependencies.
+		 *
+		 * (Automatic submenu creation might still create a submenu from
+		 * an 'if' before this code runs.)
+		 *
+		 * Before:
+		 *
+		 *	A
+		 *	if ...
+		 *	 +-B
+		 *	 +-C
+		 *	D
+		 *
+		 * After:
+		 *
+		 *	A
+		 *	if ...
+		 *	B
+		 *	C
+		 *	D
+		 */
 		if (menu->list && (!menu->prompt || !menu->prompt->text)) {
 			for (last_menu = menu->list; ; last_menu = last_menu->next) {
 				last_menu->parent = parent;
-- 
2.14.1

[PATCH 1/2] kconfig: Document 'if' flattening logic

[Ulf Magnusson]

It is not obvious that this refers to an 'if', making the code pretty
cryptic:

	if (menu->list && (!menu->prompt || !menu->prompt->text)) {

Kconfig keeps the 'if' menu nodes even after flattening. Reflect that in
the example to be accurate.

No functional changes. Only comments added.

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
---
 scripts/kconfig/menu.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 372eb5d9fef3..1f7bcceacde3 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -524,6 +524,30 @@ void menu_finalize(struct menu *parent)
 			*ep = expr_alloc_one(E_LIST, NULL);
 			(*ep)->right.sym = menu->sym;
 		}
+
+		/*
+		 * Flatten 'if' blocks, which do not specify a submenu and only
+		 * add dependencies.
+		 *
+		 * (Automatic submenu creation might still create a submenu from
+		 * an 'if' before this code runs.)
+		 *
+		 * Before:
+		 *
+		 *	A
+		 *	if ...
+		 *	 +-B
+		 *	 +-C
+		 *	D
+		 *
+		 * After:
+		 *
+		 *	A
+		 *	if ...
+		 *	B
+		 *	C
+		 *	D
+		 */
 		if (menu->list && (!menu->prompt || !menu->prompt->text)) {
 			for (last_menu = menu->list; ; last_menu = last_menu->next) {
 				last_menu->parent = parent;
-- 
2.14.1

[PATCH 2/2] kconfig: Improve auto. menu documentation accuracy

[Ulf Magnusson]

An 'if', 'menu', or 'choice' that depends on a preceding symbol will
also generate a submenu.

No functional changes. Only comments updated.

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
---
 scripts/kconfig/menu.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 1f7bcceacde3..86031dc36f7d 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -414,9 +414,10 @@ void menu_finalize(struct menu *parent)
 			menu_finalize(menu);
 	} else if (sym) {
 		/*
-		 * Automatic submenu creation. If sym, A, B, C, ..., are
-		 * consecutive symbols and A, B, C, ... all depend on sym, the
-		 * following menu structure will be created:
+		 * Automatic submenu creation. If sym is a symbol and A, B, C,
+		 * ... are consecutive items (symbols, menus, ifs, etc.) that
+		 * all depend on sym, then the following menu structure is
+		 * created:
 		 *
 		 *	sym
 		 *	 +-A
@@ -425,7 +426,7 @@ void menu_finalize(struct menu *parent)
 		 *	 ...
 		 *
 		 * This also works recursively, giving the following structure
-		 * if B depends on A:
+		 * if A is a symbol and B depends on A:
 		 *
 		 *	sym
 		 *	 +-A
-- 
2.14.1

[PATCH v2 1/2] kconfig: Document 'if' flattening logic

[Ulf Magnusson]

It is not obvious that this might refer to an 'if', making the code
pretty cryptic:

	if (menu->list && (!menu->prompt || !menu->prompt->text)) {

Kconfig keeps the 'if' menu nodes even after flattening. Reflect that in
the example to be accurate.

No functional changes. Only comments added.

Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
---
Changelog

v2:
I forgot to mention that this code also undoes automatic submenus created below
promptless symbols.

 scripts/kconfig/menu.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 53964d911708..a8af08aabfd6 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -531,6 +531,35 @@ void menu_finalize(struct menu *parent)
 			*ep = expr_alloc_one(E_LIST, NULL);
 			(*ep)->right.sym = menu->sym;
 		}
+
+		/*
+		 * This code serves two purposes:
+		 *
+		 * (1) Flattening 'if' blocks, which do not specify a submenu
+		 *     and only add dependencies.
+		 *
+		 *     (Automatic submenu creation might still create a submenu
+		 *     from an 'if' before this code runs.)
+		 *
+		 * (2) "Undoing" any automatic submenus created earlier below
+		 *     promptless symbols.
+		 *
+		 * Before:
+		 *
+		 *	A
+		 *	if ... (or promptless symbol)
+		 *	 +-B
+		 *	 +-C
+		 *	D
+		 *
+		 * After:
+		 *
+		 *	A
+		 *	if ... (or promptless symbol)
+		 *	B
+		 *	C
+		 *	D
+		 */
 		if (menu->list && (!menu->prompt || !menu->prompt->text)) {
 			for (last_menu = menu->list; ; last_menu = last_menu->next) {
 				last_menu->parent = parent;
-- 
2.14.1

Re: [PATCH 0/2] kconfig: 'if' and automatic submenu creation documentation

[Sam Ravnborg]

Hi Ulf.

On Sun, Jan 14, 2018 at 12:33:43PM +0100, Ulf Magnusson wrote:
> These together should make the automatic submenu logic a lot clearer.
> 
> Ulf Magnusson (2):
>   kconfig: Document 'if' flattening logic
>   kconfig: Improve auto. menu documentation accuracy
> 
>  scripts/kconfig/menu.c | 33 +++++++++++++++++++++++++++++----
>  1 file changed, 29 insertions(+), 4 deletions(-)

Very good with extra comments in menu_finalize.
If you can mamage to explain the logic related to select
then even better.

While on the subject on improving readability, then
menu_finalize would benefit from a split in a number of
well named and well documented helper functions.

It would be great if you could try this out.

	Sam

Re: [PATCH 0/2] kconfig: 'if' and automatic submenu creation documentation

[Ulf Magnusson]

On Sun, Jan 14, 2018 at 10:00 PM, Sam Ravnborg <sam@ravnborg.org> wrote:
> Hi Ulf.
>
> On Sun, Jan 14, 2018 at 12:33:43PM +0100, Ulf Magnusson wrote:
>> These together should make the automatic submenu logic a lot clearer.
>>
>> Ulf Magnusson (2):
>>   kconfig: Document 'if' flattening logic
>>   kconfig: Improve auto. menu documentation accuracy
>>
>>  scripts/kconfig/menu.c | 33 +++++++++++++++++++++++++++++----
>>  1 file changed, 29 insertions(+), 4 deletions(-)
>
> Very good with extra comments in menu_finalize.
> If you can mamage to explain the logic related to select
> then even better.

I added a short comment in
https://patchwork.kernel.org/patch/9986947/, which is already in
linux-next:

    /*
    * Handle selects and implies, which modify the
    * dependencies of the selected/implied symbol
    */
    if (prop->type == P_SELECT) {
    ...

There's also https://patchwork.kernel.org/patch/9983881/:

    /* Reverse dependencies through being selected by other symbols */
    struct expr_value rev_dep;

I guess it could be made more explicit. I don't want to heap on too
many patches at once though.

>
> While on the subject on improving readability, then
> menu_finalize would benefit from a split in a number of
> well named and well documented helper functions.
>
> It would be great if you could try this out.

I had a stab at it earlier, but never got done. I think the high-level
structure in Kconfiglib should work:
https://github.com/ulfalizer/Kconfiglib/blob/master/kconfiglib.py#L3990

Hopefully I'll get back to it later once the current batch of patches
has been merged. I'm just happy to have to have less mystery code in
there for now. Definitely one of the more opaque parts of Kconfig.

Cheers,
Ulf

Re: [PATCH 2/2] kconfig: Improve auto. menu documentation accuracy

[Masahiro Yamada]

2018-01-14 20:38 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> An 'if', 'menu', or 'choice' that depends on a preceding symbol will
> also generate a submenu.
>
> No functional changes. Only comments updated.
>
> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
> ---
>  scripts/kconfig/menu.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
> index 1f7bcceacde3..86031dc36f7d 100644
> --- a/scripts/kconfig/menu.c
> +++ b/scripts/kconfig/menu.c
> @@ -414,9 +414,10 @@ void menu_finalize(struct menu *parent)
>                         menu_finalize(menu);
>         } else if (sym) {
>                 /*
> -                * Automatic submenu creation. If sym, A, B, C, ..., are
> -                * consecutive symbols and A, B, C, ... all depend on sym, the
> -                * following menu structure will be created:
> +                * Automatic submenu creation. If sym is a symbol and A, B, C,
> +                * ... are consecutive items (symbols, menus, ifs, etc.) that
> +                * all depend on sym, then the following menu structure is
> +                * created:
>                  *
>                  *      sym
>                  *       +-A
> @@ -425,7 +426,7 @@ void menu_finalize(struct menu *parent)
>                  *       ...
>                  *
>                  * This also works recursively, giving the following structure
> -                * if B depends on A:
> +                * if A is a symbol and B depends on A:
>                  *
>                  *      sym
>                  *       +-A
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


I squashed this into the previous patch:
https://patchwork.kernel.org/patch/9991939/

Thanks!


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 1/2] kconfig: Document 'if' flattening logic

[Masahiro Yamada]

2018-01-14 23:49 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>:
> It is not obvious that this might refer to an 'if', making the code
> pretty cryptic:
>
>         if (menu->list && (!menu->prompt || !menu->prompt->text)) {
>
> Kconfig keeps the 'if' menu nodes even after flattening. Reflect that in
> the example to be accurate.
>
> No functional changes. Only comments added.
>
> Signed-off-by: Ulf Magnusson <ulfalizer@gmail.com>
> ---
> Changelog
>
> v2:
> I forgot to mention that this code also undoes automatic submenus created below
> promptless symbols.
>
>  scripts/kconfig/menu.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>

Applied to linux-kbuild/kconfig.  Thanks!


-- 
Best Regards
Masahiro Yamada