| Message ID | 1444249244-4592-1-git-send-email-petr.vorel@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On wo, 2015-10-07 at 22:20 +0200, Petr Vorel 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. > > Cc: Paul Bolle <pebolle@tiscali.nl> > Cc: Randy Dunlap <rdunlap@infradead.org> You actually want your Signed-off-by: line here. > --- > 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. > > Signed-off-by: Petr Vorel <petr.vorel@gmail.com> > --- You want to have that line there because the above text will be dropped by the git tools (because it comes are after a --- marker, which signals "end of commit explanation"). Note that the text, which I think was written by me, was basically a comment. Ie, because it came after that marker I used it to say things that I thought were not appropriate for a commit explanation. > Changes v1->v2: > Rewrote Paul Bolle's original implementation: removed use of > expr_gstr_print()'s output from rev_dep_gstr_print(). I admit that > adding revdep variable and wrapper function isn't a nice solution, but > it works directly with struct expr :-). > --- a/scripts/kconfig/expr.c > +++ b/scripts/kconfig/expr.c > @@ -1070,7 +1070,7 @@ struct expr *expr_simplify_unmet_dep(struct expr > -void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, > const char *), void *data, int prevtoken) > +static void expr_print_impl(struct expr *e, void (*fn)(void *, struct What does _impl stand for? > @@ -1125,9 +1125,12 @@ void expr_print(struct expr *e, void (*fn)(void > case E_OR: > - expr_print(e->left.expr, fn, data, E_OR); > - fn(data, NULL, " || "); > - expr_print(e->right.expr, fn, data, E_OR); > + expr_print_impl(e->left.expr, fn, data, E_OR, revdep); > + if (revdep) > + fn(data, NULL, "\n - "); > + else > + fn(data, NULL, " || "); > + expr_print_impl(e->right.expr, fn, data, E_OR, revdep); > break; > case E_AND: > expr_print(e->left.expr, fn, data, E_AND); Lazy question: will the "-" only be printed for the _top level_ E_OR expressions in "Selected by:" lines? Thanks, Paul Bolle -- 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
> > Signed-off-by: Petr Vorel <petr.vorel@gmail.com> > > --- > You want to have that line there because the above text will be dropped > by the git tools (because it comes are after a --- marker, which signals > "end of commit explanation"). Note that the text, which I think was > written by me, was basically a comment. Ie, because it came after that > marker I used it to say things that I thought were not appropriate for a > commit explanation. Thanks for explanation, I'll fix my Signed-off-by: in v3. And also clean up comments. > > --- a/scripts/kconfig/expr.c > > +++ b/scripts/kconfig/expr.c > > @@ -1070,7 +1070,7 @@ struct expr *expr_simplify_unmet_dep(struct expr > > -void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, > > const char *), void *data, int prevtoken) > > +static void expr_print_impl(struct expr *e, void (*fn)(void *, struct > What does _impl stand for? _impl is for implementation. I didn't want to add new parameter which is meaningless to most of expr_print() calls, so I created wrapper function. Ugly approach, I know. > > @@ -1125,9 +1125,12 @@ void expr_print(struct expr *e, void (*fn)(void > > case E_OR: > > - expr_print(e->left.expr, fn, data, E_OR); > > - fn(data, NULL, " || "); > > - expr_print(e->right.expr, fn, data, E_OR); > > + expr_print_impl(e->left.expr, fn, data, E_OR, > revdep); > > + if (revdep) > > + fn(data, NULL, "\n - "); > > + else > > + fn(data, NULL, " || "); > > + expr_print_impl(e->right.expr, fn, data, E_OR, > revdep); > > break; > > case E_AND: > > expr_print(e->left.expr, fn, data, E_AND); > Lazy question: will the "-" only be printed for the _top level_ E_OR > expressions in "Selected by:" lines? Well, I don't check for it in the code. But IMHO all non top level E_OR expressions are in parentheses, which aren't split by "-". So, yes. Anyway, do you have an example of non top level E_OR, which is not in parentheses? Kind regards, Petr -- 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
diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c index 667d1aa..af0e840 100644 --- a/scripts/kconfig/expr.c +++ b/scripts/kconfig/expr.c @@ -1070,7 +1070,7 @@ struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2) return expr_get_leftmost_symbol(ret); } -void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken) +static void expr_print_impl(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken, bool revdep) { if (!e) { fn(data, NULL, "y"); @@ -1125,9 +1125,12 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char * fn(data, e->right.sym, e->right.sym->name); break; case E_OR: - expr_print(e->left.expr, fn, data, E_OR); - fn(data, NULL, " || "); - expr_print(e->right.expr, fn, data, E_OR); + expr_print_impl(e->left.expr, fn, data, E_OR, revdep); + if (revdep) + fn(data, NULL, "\n - "); + else + fn(data, NULL, " || "); + expr_print_impl(e->right.expr, fn, data, E_OR, revdep); break; case E_AND: expr_print(e->left.expr, fn, data, E_AND); @@ -1160,6 +1163,11 @@ void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char * fn(data, NULL, ")"); } +void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken) +{ + expr_print_impl(e, fn, data, prevtoken, false); +} + static void expr_print_file_helper(void *data, struct symbol *sym, const char *str) { xfwrite(str, strlen(str), 1, data); @@ -1204,3 +1212,23 @@ void expr_gstr_print(struct expr *e, struct gstr *gs) { expr_print(e, expr_print_gstr_helper, gs, E_NONE); } + +/* + * 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. + */ +void rev_dep_gstr_print(struct expr *e, struct gstr *gs) +{ + expr_print_impl(e, expr_print_gstr_helper, gs, E_NONE, true); +} diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h index 973b6f7..f3bf88d 100644 --- a/scripts/kconfig/expr.h +++ b/scripts/kconfig/expr.h @@ -220,6 +220,7 @@ struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2); void expr_fprint(struct expr *e, FILE *out); struct gstr; /* forward */ void expr_gstr_print(struct expr *e, struct gstr *gs); +void rev_dep_gstr_print(struct expr *e, struct gstr *gs); static inline int expr_is_yes(struct expr *e) { diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c index b05cc3d..b6b383e 100644 --- a/scripts/kconfig/menu.c +++ b/scripts/kconfig/menu.c @@ -661,8 +661,8 @@ 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"); + str_append(r, "\n - "); + rev_dep_gstr_print(sym->rev_dep.expr, r); } str_append(r, "\n\n"); }
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. Cc: Paul Bolle <pebolle@tiscali.nl> Cc: Randy Dunlap <rdunlap@infradead.org> --- 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. Signed-off-by: Petr Vorel <petr.vorel@gmail.com> --- Changes v1->v2: Rewrote Paul Bolle's original implementation: removed use of expr_gstr_print()'s output from rev_dep_gstr_print(). I admit that adding revdep variable and wrapper function isn't a nice solution, but it works directly with struct expr :-). --- scripts/kconfig/expr.c | 36 ++++++++++++++++++++++++++++++++---- scripts/kconfig/expr.h | 1 + scripts/kconfig/menu.c | 4 ++-- 3 files changed, 35 insertions(+), 6 deletions(-)