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

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

[Petr Vorel]

Hi there,

I like Paul's patch, which he posted some time ago. I've been using it for a
while. How about including it?

Kind regards,
Petr

Paul Bolle (1):
  [RFC] kconfig: menuconfig make "Selected by:" readable

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

-- 
1.8.0

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

[Petr Vorel]

From: Paul Bolle <pebolle@tiscali.nl>

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

Not-yet-signed-off-by: Paul Bolle <pebolle@tiscali.nl>
Tested-by: Petr Vorel <petr.vorel@gmail.com>
Cc: Paul Bolle <pebolle@tiscali.nl>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Petr Vorel <petr.vorel@gmail.com>
---
 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 b05cc3d..a8b37a2 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 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
  */
 static void get_symbol_str(struct gstr *r, struct symbol *sym,
@@ -661,8 +741,7 @@ static 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.8.0

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

[Paul Bolle]

On ma, 2015-09-14 at 21:05 +0200, Petr Vorel wrote:
> From: Paul Bolle <pebolle@tiscali.nl>

(Petr and I have been in contact about this patch. The way it's been
resubmitted is a bit odd. That's probably because I didn't give Petr's
queries about my patch the attention they deserved.)

> This is a _hack_ to show what might be a better way to do this.

That's the reason I submitted this as an RFC.

(By now I doubt whether RFC patches are worth the effort. I'm inclined
to think that one might as well send in actual patches. Reviewing RFCs
seems to be about as much work as reviewing real patches. So why bother
reviewing them when the submitter might claim "It's only an RFC!" once
things get serious?)  

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

I did this for a reason!	

> Tested-by: Petr Vorel <petr.vorel@gmail.com>

(Resubmitting someone else's patch can be the right thing to do. The
patch should be correctly signed off in that case - this patch wasn't! -
and the person resubmitting should sign off too. Petr didn't sign off on
this patch.)

> +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);

This is the main problem with my RFC. I feel that the right thing for
 rev_dep_gstr_print() would be to work on a struct expr directly,
instead of cheating by using expr_gstr_print()'s output.

Would you mind working on a v2 that tries to do that?

> +	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);
> +}

Thanks,


Paul Bolle

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

[Michal Marek]

Dne 14.9.2015 v 21:05 Petr Vorel napsal(a):
> From: Paul Bolle <pebolle@tiscali.nl>
> 
> 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.
> ---
> 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.
> 
> Not-yet-signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> Tested-by: Petr Vorel <petr.vorel@gmail.com>
> Cc: Paul Bolle <pebolle@tiscali.nl>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Cc: Petr Vorel <petr.vorel@gmail.com>
> ---
>  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 b05cc3d..a8b37a2 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);

assert/abort?

BTW, the whole splitting can be done in a singe loop a-la

while (c = s++) {
	if (c == '(')
		lvl++;
	else if (c == ')')
		lvl--;
	else if (c == '|' && lvl == 0)
		/* insert newline and indent */
	/* output c */
}

No need to seek back and forth in the string.


> -		expr_gstr_print(sym->rev_dep.expr, r);
> -		str_append(r, "\n");
> +		rev_dep_gstr_print(sym->rev_dep.expr, r);

... and a cleaner fix would be to patch the printer, instead of
generating a string representation and converting it.

Michal

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

[Paul Bolle]

On ma, 2015-10-26 at 21:21 +0100, Michal Marek wrote:
> ... and a cleaner fix would be to patch the printer, instead of
> generating a string representation and converting it.

Petr has submitted a v2 that appears to do just that. (Actually we're
already at v3, see 
http://www.spinics.net/lists/linux-kbuild/msg11840.html .)

I haven't looked at the patch too closely (until now I mostly helped
Petr with the boring stuff, like correct sign-off). Perhaps I find time
to have a close look at v3. Whatever happens, it's rather nice to notice
that Petr stopped (politely!) bugging me to finish my RFC patch and
submitted an updated version. I really appreciate that!


Paul Bolle