public inbox for linux-kbuild@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [RFC] kconfig: menuconfig make "Selected by:" readable
@ 2015-01-21 23:00 Paul Bolle
  2015-01-22  2:23 ` Randy Dunlap
  2015-01-22  8:38 ` Paul Bolle
  0 siblings, 2 replies; 11+ messages in thread
From: Paul Bolle @ 2015-01-21 23:00 UTC (permalink / raw)
  To: Michal Marek; +Cc: linux-kbuild

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* Re: [PATCH] [RFC] kconfig: menuconfig make "Selected by:" readable
@ 2015-05-18 21:19 Petr Vorel
  2015-05-18 21:28 ` Paul Bolle
  0 siblings, 1 reply; 11+ messages in thread
From: Petr Vorel @ 2015-05-18 21:19 UTC (permalink / raw)
  To: linux-kbuild; +Cc: pebolle, mmarek

> 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-05-19  4:06 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-21 23:00 [PATCH] [RFC] kconfig: menuconfig make "Selected by:" readable Paul Bolle
2015-01-22  2:23 ` Randy Dunlap
2015-01-22  8:35   ` Paul Bolle
2015-01-22  8:50     ` Paul Bolle
2015-01-22 10:33     ` Michal Marek
2015-01-22 10:47       ` Paul Bolle
2015-01-22 10:51         ` Michal Marek
2015-01-22  8:38 ` Paul Bolle
  -- strict thread matches above, loose matches on Subject: below --
2015-05-18 21:19 Petr Vorel
2015-05-18 21:28 ` Paul Bolle
2015-05-19  4:06   ` Petr Vorel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox