[PATCH v3 1/1] kconfig: menuconfig make "selected by:" readable

[PATCH v3 1/1] kconfig: menuconfig make "selected by:" readable

[Petr Vorel]

rev_dep expressions can get rather unwieldy, especially if a symbol is
selected by more than a handful of other symbols. Ie, it's possible to
have near endless expressions like:
   A && B && !C || D || F && (G || H) || [...]

Chop these expressions into actually readable chunks:
   - A && B && !C
   - D
   - F && (G || H)
   - [...]

Ie, transform the top level "||" tokens into newlines and prepend each
line with a minus. This makes the "Selected by:" blurb much easier to
read.

This also prevents trimming too long lines.

Based on patch from Paul Bolle.

Reported-by: Paul Bolle <pebolle@tiscali.nl>
Cc: Paul Bolle <pebolle@tiscali.nl>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 scripts/kconfig/expr.c | 16 +++++++++++++---
 scripts/kconfig/menu.c |  2 +-
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
index cbf4996dd9c1..4b4309b59349 100644
--- a/scripts/kconfig/expr.c
+++ b/scripts/kconfig/expr.c
@@ -1070,6 +1070,8 @@ struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2)
 	return expr_get_leftmost_symbol(ret);
 }
 
+static int print_level = 0;
+
 void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken)
 {
 	if (!e) {
@@ -1077,8 +1079,11 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *
 		return;
 	}
 
-	if (expr_compare_type(prevtoken, e->type) > 0)
+	if (expr_compare_type(prevtoken, e->type) > 0) {
+		print_level++;
 		fn(data, NULL, "(");
+	}
+
 	switch (e->type) {
 	case E_SYMBOL:
 		if (e->left.sym->name)
@@ -1126,7 +1131,10 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *
 		break;
 	case E_OR:
 		expr_print(e->left.expr, fn, data, E_OR);
-		fn(data, NULL, " || ");
+		if (print_level == 0)
+			fn(data, NULL, "\n  - ");
+		else
+			fn(data, NULL, " || ");
 		expr_print(e->right.expr, fn, data, E_OR);
 		break;
 	case E_AND:
@@ -1156,8 +1164,10 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *
 		break;
 	  }
 	}
-	if (expr_compare_type(prevtoken, e->type) > 0)
+	if (expr_compare_type(prevtoken, e->type) > 0) {
+		print_level--;
 		fn(data, NULL, ")");
+	}
 }
 
 static void expr_print_file_helper(void *data, struct symbol *sym, const char *str)
diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index e9357931b47d..3daf7b81637f 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -675,7 +675,7 @@ static void get_symbol_str(struct gstr *r, struct symbol *sym,
 
 	get_symbol_props_str(r, sym, P_SELECT, _("  Selects: "));
 	if (sym->rev_dep.expr) {
-		str_append(r, _("  Selected by: "));
+		str_append(r, _("  Selected by: \n  - "));
 		expr_gstr_print(sym->rev_dep.expr, r);
 		str_append(r, "\n");
 	}
-- 
2.11.0

Re: [PATCH v3 1/1] kconfig: menuconfig make "selected by:" readable

[Petr Vorel]

Hi,

> rev_dep expressions can get rather unwieldy, especially if a symbol is
> selected by more than a handful of other symbols. Ie, it's possible to
> have near endless expressions like:
>    A && B && !C || D || F && (G || H) || [...]

> Chop these expressions into actually readable chunks:
>    - A && B && !C
>    - D
>    - F && (G || H)
>    - [...]

> Ie, transform the top level "||" tokens into newlines and prepend each
> line with a minus. This makes the "Selected by:" blurb much easier to
> read.

> This also prevents trimming too long lines.

> Based on patch from Paul Bolle.

> Reported-by: Paul Bolle <pebolle@tiscali.nl>
> Cc: Paul Bolle <pebolle@tiscali.nl>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
>  scripts/kconfig/expr.c | 16 +++++++++++++---
>  scripts/kconfig/menu.c |  2 +-
>  2 files changed, 14 insertions(+), 4 deletions(-)

> diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
> index cbf4996dd9c1..4b4309b59349 100644
> --- a/scripts/kconfig/expr.c
> +++ b/scripts/kconfig/expr.c
> @@ -1070,6 +1070,8 @@ struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2)
>  	return expr_get_leftmost_symbol(ret);
>  }

> +static int print_level = 0;
> +
>  void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken)
>  {
>  	if (!e) {
> @@ -1077,8 +1079,11 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *
>  		return;
>  	}

> -	if (expr_compare_type(prevtoken, e->type) > 0)
> +	if (expr_compare_type(prevtoken, e->type) > 0) {
> +		print_level++;
>  		fn(data, NULL, "(");
> +	}
> +
>  	switch (e->type) {
>  	case E_SYMBOL:
>  		if (e->left.sym->name)
> @@ -1126,7 +1131,10 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *
>  		break;
>  	case E_OR:
>  		expr_print(e->left.expr, fn, data, E_OR);
> -		fn(data, NULL, " || ");
> +		if (print_level == 0)
> +			fn(data, NULL, "\n  - ");
> +		else
> +			fn(data, NULL, " || ");
>  		expr_print(e->right.expr, fn, data, E_OR);
>  		break;
>  	case E_AND:
> @@ -1156,8 +1164,10 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *
>  		break;
>  	  }
>  	}
> -	if (expr_compare_type(prevtoken, e->type) > 0)
> +	if (expr_compare_type(prevtoken, e->type) > 0) {
> +		print_level--;
>  		fn(data, NULL, ")");
> +	}
>  }

>  static void expr_print_file_helper(void *data, struct symbol *sym, const char *str)
> diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
> index e9357931b47d..3daf7b81637f 100644
> --- a/scripts/kconfig/menu.c
> +++ b/scripts/kconfig/menu.c
> @@ -675,7 +675,7 @@ static void get_symbol_str(struct gstr *r, struct symbol *sym,

>  	get_symbol_props_str(r, sym, P_SELECT, _("  Selects: "));
>  	if (sym->rev_dep.expr) {
> -		str_append(r, _("  Selected by: "));
> +		str_append(r, _("  Selected by: \n  - "));
>  		expr_gstr_print(sym->rev_dep.expr, r);
>  		str_append(r, "\n");
>  	}

Any comment on this patch, please?


Kind regards,
Petr

Re: [PATCH v3 1/1] kconfig: menuconfig make "selected by:" readable

[Masahiro Yamada]

Hi Petr,

2017-04-03 18:35 GMT+09:00 Petr Vorel <pvorel@suse.cz>:
> Hi,
>
>> rev_dep expressions can get rather unwieldy, especially if a symbol is
>> selected by more than a handful of other symbols. Ie, it's possible to
>> have near endless expressions like:
>>    A && B && !C || D || F && (G || H) || [...]
>
>> Chop these expressions into actually readable chunks:
>>    - A && B && !C
>>    - D
>>    - F && (G || H)
>>    - [...]
>
>> Ie, transform the top level "||" tokens into newlines and prepend each
>> line with a minus. This makes the "Selected by:" blurb much easier to
>> read.
>
>> This also prevents trimming too long lines.
>
>> Based on patch from Paul Bolle.
>
>> Reported-by: Paul Bolle <pebolle@tiscali.nl>
>> Cc: Paul Bolle <pebolle@tiscali.nl>
>> Signed-off-by: Petr Vorel <pvorel@suse.cz>
>> ---
>>  scripts/kconfig/expr.c | 16 +++++++++++++---
>>  scripts/kconfig/menu.c |  2 +-
>>  2 files changed, 14 insertions(+), 4 deletions(-)
>
>> diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
>> index cbf4996dd9c1..4b4309b59349 100644
>> --- a/scripts/kconfig/expr.c
>> +++ b/scripts/kconfig/expr.c
>> @@ -1070,6 +1070,8 @@ struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2)
>>       return expr_get_leftmost_symbol(ret);
>>  }
>
>> +static int print_level = 0;
>> +
>>  void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken)
>>  {
>>       if (!e) {
>> @@ -1077,8 +1079,11 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *
>>               return;
>>       }
>
>> -     if (expr_compare_type(prevtoken, e->type) > 0)
>> +     if (expr_compare_type(prevtoken, e->type) > 0) {
>> +             print_level++;
>>               fn(data, NULL, "(");
>> +     }
>> +
>>       switch (e->type) {
>>       case E_SYMBOL:
>>               if (e->left.sym->name)
>> @@ -1126,7 +1131,10 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *
>>               break;
>>       case E_OR:
>>               expr_print(e->left.expr, fn, data, E_OR);
>> -             fn(data, NULL, " || ");
>> +             if (print_level == 0)
>> +                     fn(data, NULL, "\n  - ");
>> +             else
>> +                     fn(data, NULL, " || ");
>>               expr_print(e->right.expr, fn, data, E_OR);
>>               break;
>>       case E_AND:
>> @@ -1156,8 +1164,10 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *
>>               break;
>>         }
>>       }
>> -     if (expr_compare_type(prevtoken, e->type) > 0)
>> +     if (expr_compare_type(prevtoken, e->type) > 0) {
>> +             print_level--;
>>               fn(data, NULL, ")");
>> +     }
>>  }
>
>>  static void expr_print_file_helper(void *data, struct symbol *sym, const char *str)
>> diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
>> index e9357931b47d..3daf7b81637f 100644
>> --- a/scripts/kconfig/menu.c
>> +++ b/scripts/kconfig/menu.c
>> @@ -675,7 +675,7 @@ static void get_symbol_str(struct gstr *r, struct symbol *sym,
>
>>       get_symbol_props_str(r, sym, P_SELECT, _("  Selects: "));
>>       if (sym->rev_dep.expr) {
>> -             str_append(r, _("  Selected by: "));
>> +             str_append(r, _("  Selected by: \n  - "));
>>               expr_gstr_print(sym->rev_dep.expr, r);
>>               str_append(r, "\n");
>>       }
>
> Any comment on this patch, please?


I noticed you added me in the To: list.

Basically I think this patch is a nice idea,
but I am not a Kconfig maintainer.  Sorry.
(I could pick up a really trivial one via kbuild tree somehow,
but I want this one to be handled by a person with expertise in the area.)

Yann E. MORIN is the Kconfig maintainer, but he has been
silent for a few years. I guess Kconfig needs a new maintainer.


Please take my comment just as a Kconfig user's point of view.

The git-log only mentions "select", but I notice this patch
also changes "depends on" format in a bit different way.

For example,

General setup  --->
     Kernel compression mode (Gzip)  --->


shows its help like this:

  Depends on: HAVE_KERNEL_GZIP  [=y]
  - HAVE_KERNEL_BZIP2 [=n]
  - HAVE_KERNEL_LZMA [=y]
  - HAVE_KERNEL_XZ [=y]
  - HAVE_KERNEL_LZO [=y]
  - HAVE_KERNEL_LZ4 [=y]

(the first dependency in the same line in the "Depends on")



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v3 1/1] kconfig: menuconfig make "selected by:" readable

[Petr Vorel]

Hi Masahiro,

> Hi Petr,

> 2017-04-03 18:35 GMT+09:00 Petr Vorel <pvorel@suse.cz>:
> > Hi,

> >> rev_dep expressions can get rather unwieldy, especially if a symbol is
> >> selected by more than a handful of other symbols. Ie, it's possible to
> >> have near endless expressions like:
> >>    A && B && !C || D || F && (G || H) || [...]

> >> Chop these expressions into actually readable chunks:
> >>    - A && B && !C
> >>    - D
> >>    - F && (G || H)
> >>    - [...]

> >> Ie, transform the top level "||" tokens into newlines and prepend each
> >> line with a minus. This makes the "Selected by:" blurb much easier to
> >> read.

> >> This also prevents trimming too long lines.

> >> Based on patch from Paul Bolle.
<snip>

> > Any comment on this patch, please?


> I noticed you added me in the To: list.
Thank you for your comments.
I'm sorry, it took me rather long time to reply.

> Basically I think this patch is a nice idea,
> but I am not a Kconfig maintainer.  Sorry.
> (I could pick up a really trivial one via kbuild tree somehow,
> but I want this one to be handled by a person with expertise in the area.)

> Yann E. MORIN is the Kconfig maintainer, but he has been
> silent for a few years. I guess Kconfig needs a new maintainer.
Well, Yann is still listed in MAINTAINERS file, even he's inactive (according to the date
of his git tree).


> Please take my comment just as a Kconfig user's point of view.

> The git-log only mentions "select", but I notice this patch
> also changes "depends on" format in a bit different way.

> For example,

> General setup  --->
>      Kernel compression mode (Gzip)  --->


> shows its help like this:

>   Depends on: HAVE_KERNEL_GZIP  [=y]
>   - HAVE_KERNEL_BZIP2 [=n]
>   - HAVE_KERNEL_LZMA [=y]
>   - HAVE_KERNEL_XZ [=y]
>   - HAVE_KERNEL_LZO [=y]
>   - HAVE_KERNEL_LZ4 [=y]

> (the first dependency in the same line in the "Depends on")

You're right, I overlooked it. Do you think it makes sense use it also for "Depends on"
section?


Kind regards,
Petr

Re: [PATCH v3 1/1] kconfig: menuconfig make "selected by:" readable

[Yann E. MORIN]

Petr, All,

On 2017-09-20 16:26 +0200, Petr Vorel spake thusly:
> > 2017-04-03 18:35 GMT+09:00 Petr Vorel <pvorel@suse.cz>:
> > Yann E. MORIN is the Kconfig maintainer, but he has been
> > silent for a few years. I guess Kconfig needs a new maintainer.
> Well, Yann is still listed in MAINTAINERS file, even he's inactive (according to the date
> of his git tree).

I already sent a patch relinquishing maintainership a while ago:
    https://lkml.org/lkml/2017/4/10/1096

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Re: [PATCH v3 1/1] kconfig: menuconfig make "selected by:" readable

[Masahiro Yamada]

Hi Yann

2017-09-21 1:38 GMT+09:00 Yann E. MORIN <yann.morin.1998@free.fr>:
> Petr, All,
>
> On 2017-09-20 16:26 +0200, Petr Vorel spake thusly:
>> > 2017-04-03 18:35 GMT+09:00 Petr Vorel <pvorel@suse.cz>:
>> > Yann E. MORIN is the Kconfig maintainer, but he has been
>> > silent for a few years. I guess Kconfig needs a new maintainer.
>> Well, Yann is still listed in MAINTAINERS file, even he's inactive (according to the date
>> of his git tree).
>
> I already sent a patch relinquishing maintainership a while ago:
>     https://lkml.org/lkml/2017/4/10/1096
>

Can you send it to Linus to pick it up directly?



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v3 1/1] kconfig: menuconfig make "selected by:" readable

[Masahiro Yamada]

Hi Petr,

2017-09-20 23:26 GMT+09:00 Petr Vorel <petr.vorel@gmail.com>:
> Hi Masahiro,
>
>> Hi Petr,
>
>> 2017-04-03 18:35 GMT+09:00 Petr Vorel <pvorel@suse.cz>:
>> > Hi,
>
>> >> rev_dep expressions can get rather unwieldy, especially if a symbol is
>> >> selected by more than a handful of other symbols. Ie, it's possible to
>> >> have near endless expressions like:
>> >>    A && B && !C || D || F && (G || H) || [...]
>
>> >> Chop these expressions into actually readable chunks:
>> >>    - A && B && !C
>> >>    - D
>> >>    - F && (G || H)
>> >>    - [...]
>
>> >> Ie, transform the top level "||" tokens into newlines and prepend each
>> >> line with a minus. This makes the "Selected by:" blurb much easier to
>> >> read.
>
>> >> This also prevents trimming too long lines.
>
>> >> Based on patch from Paul Bolle.
> <snip>
>
>> > Any comment on this patch, please?
>
>
>> I noticed you added me in the To: list.
> Thank you for your comments.
> I'm sorry, it took me rather long time to reply.
>
>> Basically I think this patch is a nice idea,
>> but I am not a Kconfig maintainer.  Sorry.
>> (I could pick up a really trivial one via kbuild tree somehow,
>> but I want this one to be handled by a person with expertise in the area.)
>
>> Yann E. MORIN is the Kconfig maintainer, but he has been
>> silent for a few years. I guess Kconfig needs a new maintainer.
> Well, Yann is still listed in MAINTAINERS file, even he's inactive (according to the date
> of his git tree).
>
>
>> Please take my comment just as a Kconfig user's point of view.
>
>> The git-log only mentions "select", but I notice this patch
>> also changes "depends on" format in a bit different way.
>
>> For example,
>
>> General setup  --->
>>      Kernel compression mode (Gzip)  --->
>
>
>> shows its help like this:
>
>>   Depends on: HAVE_KERNEL_GZIP  [=y]
>>   - HAVE_KERNEL_BZIP2 [=n]
>>   - HAVE_KERNEL_LZMA [=y]
>>   - HAVE_KERNEL_XZ [=y]
>>   - HAVE_KERNEL_LZO [=y]
>>   - HAVE_KERNEL_LZ4 [=y]
>
>> (the first dependency in the same line in the "Depends on")
>
> You're right, I overlooked it. Do you think it makes sense use it also for "Depends on"
> section?

Yeah, I guess printing it in a consistent way will be better.






-- 
Best Regards
Masahiro Yamada