[PATCH] [RFC] kconfig: menuconfig make "Selected by:" readable

[PATCH] [RFC] kconfig: menuconfig make "Selected by:" readable

[Paul Bolle]

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.

Not-yet-signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
Today I found myself wondering why a certain Kconfig was selected.
Currently menuconfig's help is of no use in complicated cases. Please
look at the help of USB or CRYPTO to see what I mean.

This is a _hack_ to show what might be a better way to do this. It
parses a stringified version of the reverse dependency, and not the
actual reverse dependecy expression. But that was easier to cobble
together.

One cool improvement would be to change to minus in front of the
subexpressions to Y or M for those that actually set the symbol. Anyhow,
other suggestions and feedback is welcome.

 scripts/kconfig/menu.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 81 insertions(+), 2 deletions(-)

diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 72c9dba84c5d..eb73fe77513e 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -613,6 +613,86 @@ static struct property *get_symbol_prop(struct symbol *sym)
 }
 
 /*
+ * Assuming we're just past an opening parenthesis in a NUL terminated string,
+ * find it's closing parenthesis and return its postion. Die otherwise.
+ */
+static const char *matching_paren(const char *s)
+{
+	int lvl = 1;
+
+	while (1) {
+		if (*s == '(')
+			lvl++;
+		else if (*s == ')')
+			lvl--;
+		if (lvl == 0)
+			break;
+		if (*s == '\0')
+			/* huh? */
+			exit(1);
+		s++;
+	}
+
+	return s;
+}
+
+/*
+ * 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.
+ */
+static void rev_dep_gstr_print(struct gstr *gs, struct expr *e)
+{
+	struct gstr tmp = str_new();
+	const char *prev, *start;
+	char *beam;
+
+	expr_gstr_print(e, &tmp);
+	prev = start = str_get(&tmp);
+
+	str_append(gs, "\n  - ");
+
+	while ((beam = index(start, '|'))) {
+		char *lparen = index(start, '(');
+
+		/* don't split "(I || J)" */
+		if (lparen && (lparen < beam)) {
+			const char *rparen = matching_paren(++lparen);
+
+			/* skip the expression inside parentheses */
+			start = ++rparen;
+			continue;
+		}
+
+		/* we can assume we're fed a sane string, so the space before
+		 * the beam gets turned into a NUL */
+		*(beam - 1) = '\0';
+		str_append(gs, prev);
+		str_append(gs, "\n  - ");
+		/* assume sane string, so skip the second beam */
+		beam++;
+		/* trim */
+		while (*++beam == ' ')
+			;
+		prev = start = beam;
+	}
+
+	str_append(gs, prev);
+
+	str_free(&tmp);
+}
+
+/*
  * head is optional and may be NULL
  */
 void get_symbol_str(struct gstr *r, struct symbol *sym,
@@ -661,8 +741,7 @@ void get_symbol_str(struct gstr *r, struct symbol *sym,
 		str_append(r, "\n");
 	if (sym->rev_dep.expr) {
 		str_append(r, _("  Selected by: "));
-		expr_gstr_print(sym->rev_dep.expr, r);
-		str_append(r, "\n");
+		rev_dep_gstr_print(sym->rev_dep.expr, r);
 	}
 	str_append(r, "\n\n");
 }
-- 
1.9.3

Re: [PATCH] [RFC] kconfig: menuconfig make "Selected by:" readable

[Randy Dunlap]

On 01/21/15 15:00, Paul Bolle wrote:
> 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.

I agree that something like this needs to be done.

After reading these expressions for a few years, I can handle them OK, but
I expect that I look at them more than most people do (with the exception of Paul).

Here is another confusing one, at least to me:
[This is for LBDAF on a 64-biy x86 kernel config.]

Depends on: BLOCK [=y] && !64BIT [=y]

I guess that the value [y,m,n] inside the square brackets is always(?) the
symbol value (64BIT) and not the expression value (!64BIT), but that isn't
always clear IMO.


> Not-yet-signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> ---
> Today I found myself wondering why a certain Kconfig was selected.
> Currently menuconfig's help is of no use in complicated cases. Please
> look at the help of USB or CRYPTO to see what I mean.
> 
> This is a _hack_ to show what might be a better way to do this. It
> parses a stringified version of the reverse dependency, and not the
> actual reverse dependecy expression. But that was easier to cobble
> together.
> 
> One cool improvement would be to change to minus in front of the
> subexpressions to Y or M for those that actually set the symbol. Anyhow,
> other suggestions and feedback is welcome.
> 
>  scripts/kconfig/menu.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 81 insertions(+), 2 deletions(-)


-- 
~Randy

Re: [PATCH] [RFC] kconfig: menuconfig make "Selected by:" readable

[Paul Bolle]

On Wed, 2015-01-21 at 18:23 -0800, Randy Dunlap wrote:
> On 01/21/15 15:00, Paul Bolle wrote:
> > 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.
> 
> I agree that something like this needs to be done.
> 
> After reading these expressions for a few years, I can handle them OK, but
> I expect that I look at them more than most people do (with the exception of Paul).

That's a problem with a broader scope. The specific problem this RFC
targets is neatly demonstrated by this screenshot: 
    Symbol: SERIAL_CORE_CONSOLE [=n]
    Type  : boolean
     Defined at drivers/tty/serial/Kconfig:805
  Depends on: TTY [=n] && HAS_IOMEM [=y]
  Selected by: SERIAL_8250_CONSOLE [=n] && TTY [=n] && HAS_IOMEM [=y] && SERIAL_8250 [=n]=y || SERIAL_AMBA_PL010_CONSOLE [=n] && [...]

That line ends about two meters to the right of your screen. Really.

> Here is another confusing one, at least to me:
> [This is for LBDAF on a 64-biy x86 kernel config.]
> 
> Depends on: BLOCK [=y] && !64BIT [=y]
> 
> I guess that the value [y,m,n] inside the square brackets is always(?) the
> symbol value (64BIT) and not the expression value (!64BIT), but that isn't
> always clear IMO.

I think you got that right. I guess the entire screen for LBDAF looked
like this _hand edited_ example:
    Symbol: LBDAF [=n]                                      
    Type  : boolean                                         
    Prompt: Support for large (2TB+) block devices and files
      Location:                                             
    (1) -> Enable the block layer (BLOCK [=y])              
      Defined at block/Kconfig:26                           
      Depends on: BLOCK [=y] && !64BIT [=y]                 

In this case 64BIT is set to 'y' (otherwise LBDAF would have been 'y').
This isn't a bug issue, of course, but I still can see how this can be
confusing. Perhaps the last line should read:
      Unmet dependency on: BLOCK [=y] && !64BIT [=y]

Would that help? Or would 
      Depends on: BLOCK [=y] && !64BIT [=n]

(ie, print the value if "!64BIT") be clearer?

> > Not-yet-signed-off-by: Paul Bolle <pebolle@tiscali.nl>

I'm glad I added that!

> > Today I found myself wondering why a certain Kconfig was selected.
> > Currently menuconfig's help is of no use in complicated cases. Please
> > look at the help of USB or CRYPTO to see what I mean.
> > 
> > This is a _hack_ to show what might be a better way to do this. It
> > parses a stringified version of the reverse dependency, and not the
> > actual reverse dependecy expression. But that was easier to cobble
> > together.
> > 
> > One cool improvement would be to change to minus in front of the
> > subexpressions to Y or M for those that actually set the symbol. Anyhow,
> > other suggestions and feedback is welcome.
> > 
> >  scripts/kconfig/menu.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 81 insertions(+), 2 deletions(-)


Paul Bolle

Re: [PATCH] [RFC] kconfig: menuconfig make "Selected by:" readable

[Paul Bolle]

On Thu, 2015-01-22 at 00:00 +0100, Paul Bolle wrote:
> 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.
> 
> Not-yet-signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> ---
> Today I found myself wondering why a certain Kconfig was selected.
> Currently menuconfig's help is of no use in complicated cases. Please
> look at the help of USB or CRYPTO to see what I mean.
> 
> This is a _hack_ to show what might be a better way to do this. It
> parses a stringified version of the reverse dependency, and not the
> actual reverse dependecy expression. But that was easier to cobble
> together.
> 
> One cool improvement would be to change to minus in front of the
> subexpressions to Y or M for those that actually set the symbol. Anyhow,
> other suggestions and feedback is welcome.
> 
>  scripts/kconfig/menu.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 81 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
> index 72c9dba84c5d..eb73fe77513e 100644
> --- a/scripts/kconfig/menu.c
> +++ b/scripts/kconfig/menu.c
> @@ -613,6 +613,86 @@ static struct property *get_symbol_prop(struct symbol *sym)
>  }
>  
>  /*
> + * Assuming we're just past an opening parenthesis in a NUL terminated string,
> + * find it's closing parenthesis and return its postion. Die otherwise.
> + */
> +static const char *matching_paren(const char *s)
> +{
> +	int lvl = 1;
> +
> +	while (1) {
> +		if (*s == '(')
> +			lvl++;
> +		else if (*s == ')')
> +			lvl--;
> +		if (lvl == 0)
> +			break;
> +		if (*s == '\0')
> +			/* huh? */
> +			exit(1);
> +		s++;
> +	}
> +
> +	return s;
> +}
> +
> +/*
> + * 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.
> + */
> +static void rev_dep_gstr_print(struct gstr *gs, struct expr *e)

Charge: posting just before suspending machine and self. Verdict:
guilty.

That should have been
    static void rev_dep_gstr_print(struct expr *e, struct gstr *gs)

> +{
> +	struct gstr tmp = str_new();
> +	const char *prev, *start;
> +	char *beam;
> +
> +	expr_gstr_print(e, &tmp);
> +	prev = start = str_get(&tmp);
> +
> +	str_append(gs, "\n  - ");
> +
> +	while ((beam = index(start, '|'))) {
> +		char *lparen = index(start, '(');
> +
> +		/* don't split "(I || J)" */
> +		if (lparen && (lparen < beam)) {
> +			const char *rparen = matching_paren(++lparen);
> +
> +			/* skip the expression inside parentheses */
> +			start = ++rparen;
> +			continue;
> +		}
> +
> +		/* we can assume we're fed a sane string, so the space before
> +		 * the beam gets turned into a NUL */
> +		*(beam - 1) = '\0';
> +		str_append(gs, prev);
> +		str_append(gs, "\n  - ");
> +		/* assume sane string, so skip the second beam */
> +		beam++;
> +		/* trim */
> +		while (*++beam == ' ')
> +			;
> +		prev = start = beam;
> +	}
> +
> +	str_append(gs, prev);
> +
> +	str_free(&tmp);
> +}
> +
> +/*
>   * head is optional and may be NULL
>   */
>  void get_symbol_str(struct gstr *r, struct symbol *sym,
> @@ -661,8 +741,7 @@ void get_symbol_str(struct gstr *r, struct symbol *sym,
>  		str_append(r, "\n");
>  	if (sym->rev_dep.expr) {
>  		str_append(r, _("  Selected by: "));
> -		expr_gstr_print(sym->rev_dep.expr, r);
> -		str_append(r, "\n");
> +		rev_dep_gstr_print(sym->rev_dep.expr, r);
>  	}
>  	str_append(r, "\n\n");
>  }


Paul Bolle

Re: [PATCH] [RFC] kconfig: menuconfig make "Selected by:" readable

[Paul Bolle]

On Thu, 2015-01-22 at 09:35 +0100, Paul Bolle wrote:
> On Wed, 2015-01-21 at 18:23 -0800, Randy Dunlap wrote:
> > On 01/21/15 15:00, Paul Bolle wrote:
> > > 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.
> > 
> > I agree that something like this needs to be done.
> > 
> > After reading these expressions for a few years, I can handle them OK, but
> > I expect that I look at them more than most people do (with the exception of Paul).
> 
> That's a problem with a broader scope. The specific problem this RFC
> targets is neatly demonstrated by this screenshot: 
>     Symbol: SERIAL_CORE_CONSOLE [=n]
>     Type  : boolean
>      Defined at drivers/tty/serial/Kconfig:805
>   Depends on: TTY [=n] && HAS_IOMEM [=y]
>   Selected by: SERIAL_8250_CONSOLE [=n] && TTY [=n] && HAS_IOMEM [=y] && SERIAL_8250 [=n]=y || SERIAL_AMBA_PL010_CONSOLE [=n] && [...]
> 
> That line ends about two meters to the right of your screen. Really.
> 
> > Here is another confusing one, at least to me:
> > [This is for LBDAF on a 64-biy x86 kernel config.]
> > 
> > Depends on: BLOCK [=y] && !64BIT [=y]
> > 
> > I guess that the value [y,m,n] inside the square brackets is always(?) the
> > symbol value (64BIT) and not the expression value (!64BIT), but that isn't
> > always clear IMO.
> 
> I think you got that right. I guess the entire screen for LBDAF looked
> like this _hand edited_ example:
>     Symbol: LBDAF [=n]                                      
>     Type  : boolean                                         
>     Prompt: Support for large (2TB+) block devices and files
>       Location:                                             
>     (1) -> Enable the block layer (BLOCK [=y])              
>       Defined at block/Kconfig:26                           
>       Depends on: BLOCK [=y] && !64BIT [=y]                 
> 
> In this case 64BIT is set to 'y' (otherwise LBDAF would have been 'y').

This is not correct at all. If 64BIT == 'n', LBDAF can be both 'y' or
'n'. Where's that brown paper bag again?

> This isn't a bug issue, of course, but I still can see how this can be
> confusing. Perhaps the last line should read:
>       Unmet dependency on: BLOCK [=y] && !64BIT [=y]
> 
> Would that help? Or would 
>       Depends on: BLOCK [=y] && !64BIT [=n]
> 
> (ie, print the value if "!64BIT") be clearer?
> 
> > > Not-yet-signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> 
> I'm glad I added that!
> 
> > > Today I found myself wondering why a certain Kconfig was selected.
> > > Currently menuconfig's help is of no use in complicated cases. Please
> > > look at the help of USB or CRYPTO to see what I mean.
> > > 
> > > This is a _hack_ to show what might be a better way to do this. It
> > > parses a stringified version of the reverse dependency, and not the
> > > actual reverse dependecy expression. But that was easier to cobble
> > > together.
> > > 
> > > One cool improvement would be to change to minus in front of the
> > > subexpressions to Y or M for those that actually set the symbol. Anyhow,
> > > other suggestions and feedback is welcome.
> > > 
> > >  scripts/kconfig/menu.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++--
> > >  1 file changed, 81 insertions(+), 2 deletions(-)


Paul Bolle

Re: [PATCH] [RFC] kconfig: menuconfig make "Selected by:" readable

[Michal Marek]

On 2015-01-22 09:35, Paul Bolle wrote:
> In this case 64BIT is set to 'y' (otherwise LBDAF would have been 'y').
> This isn't a bug issue, of course, but I still can see how this can be
> confusing. Perhaps the last line should read:
>       Unmet dependency on: BLOCK [=y] && !64BIT [=y]
> 
> Would that help? Or would 
>       Depends on: BLOCK [=y] && !64BIT [=n]
> 
> (ie, print the value if "!64BIT") be clearer?

How about

  Depends on: (BLOCK [=y] && !64BIT [=y]) [=n]

?

Michal

Re: [PATCH] [RFC] kconfig: menuconfig make "Selected by:" readable

[Paul Bolle]

On Thu, 2015-01-22 at 11:33 +0100, Michal Marek wrote:
> On 2015-01-22 09:35, Paul Bolle wrote:
> > In this case 64BIT is set to 'y' (otherwise LBDAF would have been 'y').
> > This isn't a bug issue, of course, but I still can see how this can be
> > confusing. Perhaps the last line should read:
> >       Unmet dependency on: BLOCK [=y] && !64BIT [=y]
> > 
> > Would that help? Or would 
> >       Depends on: BLOCK [=y] && !64BIT [=n]
> > 
> > (ie, print the value if "!64BIT") be clearer?
> 
> How about
> 
>   Depends on: (BLOCK [=y] && !64BIT [=y]) [=n]
> 
> ?

Or
   Depends on: BLOCK [=y] && !64BIT [=y] => [=n]

Whatever, we'll figure out something.

This is a curses UI, isn't it? Could we use color to distinguish the
symbols or sub-expressions that are set correctly, for that particular
dependency, from those that are not? 


Paul Bolle

Re: [PATCH] [RFC] kconfig: menuconfig make "Selected by:" readable

[Michal Marek]

On 2015-01-22 11:47, Paul Bolle wrote:
> On Thu, 2015-01-22 at 11:33 +0100, Michal Marek wrote:
>> On 2015-01-22 09:35, Paul Bolle wrote:
>>> In this case 64BIT is set to 'y' (otherwise LBDAF would have been 'y').
>>> This isn't a bug issue, of course, but I still can see how this can be
>>> confusing. Perhaps the last line should read:
>>>       Unmet dependency on: BLOCK [=y] && !64BIT [=y]
>>>
>>> Would that help? Or would 
>>>       Depends on: BLOCK [=y] && !64BIT [=n]
>>>
>>> (ie, print the value if "!64BIT") be clearer?
>>
>> How about
>>
>>   Depends on: (BLOCK [=y] && !64BIT [=y]) [=n]
>>
>> ?
> 
> Or
>    Depends on: BLOCK [=y] && !64BIT [=y] => [=n]
> 
> Whatever, we'll figure out something.
> 
> This is a curses UI, isn't it? Could we use color to distinguish the
> symbols or sub-expressions that are set correctly, for that particular
> dependency, from those that are not?

That might be a bit tricky, since there are multiple UIs and we use the
same functions to build the displayed strings.

Michal

Re: [PATCH] [RFC] kconfig: menuconfig make "Selected by:" readable

[Petr Vorel]

Hi Paul,

> There are quite a few Kconfig related things that I should put more
> effort into. Let's call the sum of those my "social debt". I'm not sure
> how I'll handle that debt, but feel free to prod me in a few weeks about
> the status of this idea. Perhaps that will help me at least handle this
> part.
Sure, I will :-). Thanks for your effort.

Petr

Re: [PATCH] [RFC] kconfig: menuconfig make "Selected by:" readable

[Paul Bolle]

Hi Petr,

On Mon, 2015-05-18 at 23:19 +0200, Petr Vorel wrote:
> This is very nice improvement, thanks for that :-). I use it happily on some old kernel.
> Although it's just a draft, it would be nice to have it in mainline.

That's nice to hear.

There are quite a few Kconfig related things that I should put more
effort into. Let's call the sum of those my "social debt". I'm not sure
how I'll handle that debt, but feel free to prod me in a few weeks about
the status of this idea. Perhaps that will help me at least handle this
part.

Thanks,


Paul Bolle

Re: [PATCH] [RFC] kconfig: menuconfig make "Selected by:" readable

[Petr Vorel]

> On Thu, 2015-01-22 at 00:00 +0100, Paul Bolle wrote:
> > 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.

> > Not-yet-signed-off-by: Paul Bolle <pebolle@xxxxxxxxxx>
> > ---
> > Today I found myself wondering why a certain Kconfig was selected.
> > Currently menuconfig's help is of no use in complicated cases. Please
> > look at the help of USB or CRYPTO to see what I mean.

> > This is a _hack_ to show what might be a better way to do this. It
> > parses a stringified version of the reverse dependency, and not the
> > actual reverse dependecy expression. But that was easier to cobble
> > together.

> > One cool improvement would be to change to minus in front of the
> > subexpressions to Y or M for those that actually set the symbol. Anyhow,
> > other suggestions and feedback is welcome.

> >  scripts/kconfig/menu.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 81 insertions(+), 2 deletions(-)

> > diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
> > index 72c9dba84c5d..eb73fe77513e 100644
> > --- a/scripts/kconfig/menu.c
> > +++ b/scripts/kconfig/menu.c
> > @@ -613,6 +613,86 @@ static struct property *get_symbol_prop(struct symbol *sym)
> >  }

> >  /*
> > + * Assuming we're just past an opening parenthesis in a NUL terminated string,
> > + * find it's closing parenthesis and return its postion. Die otherwise.
> > + */
> > +static const char *matching_paren(const char *s)
> > +{
> > +	int lvl = 1;
> > +
> > +	while (1) {
> > +		if (*s == '(')
> > +			lvl++;
> > +		else if (*s == ')')
> > +			lvl--;
> > +		if (lvl == 0)
> > +			break;
> > +		if (*s == '\0')
> > +			/* huh? */
> > +			exit(1);
> > +		s++;
> > +	}
> > +
> > +	return s;
> > +}
> > +
> > +/*
> > + * 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.
> > + */
> > +static void rev_dep_gstr_print(struct gstr *gs, struct expr *e)

> Charge: posting just before suspending machine and self. Verdict:
> guilty.

> That should have been
>     static void rev_dep_gstr_print(struct expr *e, struct gstr *gs)

> > +{
> > +	struct gstr tmp = str_new();
> > +	const char *prev, *start;
> > +	char *beam;
> > +
> > +	expr_gstr_print(e, &tmp);
> > +	prev = start = str_get(&tmp);
> > +
> > +	str_append(gs, "\n  - ");
> > +
> > +	while ((beam = index(start, '|'))) {
> > +		char *lparen = index(start, '(');
> > +
> > +		/* don't split "(I || J)" */
> > +		if (lparen && (lparen < beam)) {
> > +			const char *rparen = matching_paren(++lparen);
> > +
> > +			/* skip the expression inside parentheses */
> > +			start = ++rparen;
> > +			continue;
> > +		}
> > +
> > +		/* we can assume we're fed a sane string, so the space before
> > +		 * the beam gets turned into a NUL */
> > +		*(beam - 1) = '\0';
> > +		str_append(gs, prev);
> > +		str_append(gs, "\n  - ");
> > +		/* assume sane string, so skip the second beam */
> > +		beam++;
> > +		/* trim */
> > +		while (*++beam == ' ')
> > +			;
> > +		prev = start = beam;
> > +	}
> > +
> > +	str_append(gs, prev);
> > +
> > +	str_free(&tmp);
> > +}
> > +
> > +/*
> >   * head is optional and may be NULL
> >   */
> >  void get_symbol_str(struct gstr *r, struct symbol *sym,
> > @@ -661,8 +741,7 @@ void get_symbol_str(struct gstr *r, struct symbol *sym,
> >  		str_append(r, "\n");
> >  	if (sym->rev_dep.expr) {
> >  		str_append(r, _("  Selected by: "));
> > -		expr_gstr_print(sym->rev_dep.expr, r);
> > -		str_append(r, "\n");
> > +		rev_dep_gstr_print(sym->rev_dep.expr, r);
> >  	}
> >  	str_append(r, "\n\n");
> >  }


> Paul Bolle

Hi Paul,

This is very nice improvement, thanks for that :-). I use it happily on some old kernel.
Although it's just a draft, it would be nice to have it in mainline.

Kind regards,
Petr