| Message ID | 20180115230930.8723-1-petr.vorel@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/15/18 15:09, Petr Vorel wrote: > rev_dep expressions can get rather unwieldy, especially if a symbol is > selected by more than a handful of other symbols. I.e. 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) > - [...] > > I.e. transform the top level OR tokens into newlines and prepend each > line with a minus. This makes the "Selected by:" blurb much easier to > read. For consistency the same is done for "Depends on:" although the > problem isn't that bad for it. This is done only if there is more than > one top level OR. > > This also prevents trimming too long lines. > > Based on idea from Paul Bolle. > > Reported-by: Paul Bolle <pebolle@tiscali.nl> > Signed-off-by: Petr Vorel <petr.vorel@gmail.com> > --- > Tested on search USB and General setup -> Kernel compression mode (Gzip) -> Help. > > Changes v3->v4: > * Add split also for "Depends on:". This is done for consistency (reported > by Masahiro Yamada). I didn't consider other symbols which call expr_gstr_print() > as reasonable candidates (i.e. "Implies:" and "Range : "). > * Add new line and hyphen only when there are more top level OR than one. > --- > scripts/kconfig/expr.c | 16 +++++++++++++--- > scripts/kconfig/menu.c | 4 ++++ > 2 files changed, 17 insertions(+), 3 deletions(-) > Acked-by: Randy Dunlap <rdunlap@infradead.org> Tested-by: Randy Dunlap <rdunlap@infradead.org> Thanks.
2018-01-16 8:09 GMT+09:00 Petr Vorel <petr.vorel@gmail.com>: > rev_dep expressions can get rather unwieldy, especially if a symbol is > selected by more than a handful of other symbols. I.e. 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) > - [...] > > I.e. transform the top level OR tokens into newlines and prepend each > line with a minus. This makes the "Selected by:" blurb much easier to > read. For consistency the same is done for "Depends on:" although the > problem isn't that bad for it. This is done only if there is more than > one top level OR. > > This also prevents trimming too long lines. > > Based on idea from Paul Bolle. > > Reported-by: Paul Bolle <pebolle@tiscali.nl> > Signed-off-by: Petr Vorel <petr.vorel@gmail.com> > --- > Tested on search USB and General setup -> Kernel compression mode (Gzip) -> Help. > > Changes v3->v4: > * Add split also for "Depends on:". This is done for consistency (reported > by Masahiro Yamada). I didn't consider other symbols which call expr_gstr_print() > as reasonable candidates (i.e. "Implies:" and "Range : "). > * Add new line and hyphen only when there are more top level OR than one. > --- How much did you check the call graph of expr_print()? This function is called from various places. Probably, the format will be broken here are there. For example, here is an example of breakage. -----------------(test case)------------- config FOO bool default y select BAR config BAR bool "foo" depends on BAZ || QUX -----------------(test case end)---------- Without your patch, $ make alldefconfig scripts/kconfig/conf --alldefconfig Kconfig warning: (FOO) selects BAR which has unmet direct dependencies (BAZ || QUX) # # configuration written to .config # Then, with your patch, $ make alldefconfig scripts/kconfig/conf --alldefconfig Kconfig warning: (FOO) selects BAR which has unmet direct dependencies (BAZ - QUX) # # configuration written to .config # One more thing. This patch improves the readability of "selected by" and "implied by", but it is questionable for "depends on". Depends on: - BAR [=y] - BAZ [=y] ... means "depends on BAR && BAZ", but it might be confusing with depends on BAR depends on BAZ ... which is equivalent to "depends on BAR || BAZ". The idea seems useful, but I want to see this only where appropriate.
2018-01-23 0:04 GMT+09:00 Masahiro Yamada <yamada.masahiro@socionext.com>: > > Depends on: > - BAR [=y] > - BAZ [=y] > > ... means "depends on BAR && BAZ", > but it might be confusing with > > depends on BAR > depends on BAZ > > ... which is equivalent to "depends on BAR || BAZ". > > Sorry, my previous comment was wrong. Let me correct it. Depends on: - BAR [=y] - BAZ [=y] ... means "depends on BAR || BAZ", but it might be confusing with depends on BAR depends on BAZ ... which is equivalent to "depends on BAR && BAZ".
Hi Masahiro, > Depends on: > - BAR [=y] > - BAZ [=y] > ... means "depends on BAR || BAZ", > but it might be confusing with > depends on BAR > depends on BAZ > ... which is equivalent to "depends on BAR && BAZ". I'm going to post v5 where I change only "Selected by:" and "Implied by:". 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 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..579ed9df1409 100644 --- a/scripts/kconfig/menu.c +++ b/scripts/kconfig/menu.c @@ -668,6 +668,8 @@ static void get_symbol_str(struct gstr *r, struct symbol *sym, prop->menu->lineno); if (!expr_is_yes(prop->visible.expr)) { str_append(r, _(" Depends on: ")); + if (prop->visible.expr->type == E_OR) + str_append(r, "\n - "); expr_gstr_print(prop->visible.expr, r); str_append(r, "\n"); } @@ -676,6 +678,8 @@ 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: ")); + if (sym->rev_dep.expr->type == E_OR) + str_append(r, "\n - "); expr_gstr_print(sym->rev_dep.expr, r); str_append(r, "\n"); }
rev_dep expressions can get rather unwieldy, especially if a symbol is selected by more than a handful of other symbols. I.e. 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) - [...] I.e. transform the top level OR tokens into newlines and prepend each line with a minus. This makes the "Selected by:" blurb much easier to read. For consistency the same is done for "Depends on:" although the problem isn't that bad for it. This is done only if there is more than one top level OR. This also prevents trimming too long lines. Based on idea from Paul Bolle. Reported-by: Paul Bolle <pebolle@tiscali.nl> Signed-off-by: Petr Vorel <petr.vorel@gmail.com> --- Tested on search USB and General setup -> Kernel compression mode (Gzip) -> Help. Changes v3->v4: * Add split also for "Depends on:". This is done for consistency (reported by Masahiro Yamada). I didn't consider other symbols which call expr_gstr_print() as reasonable candidates (i.e. "Implies:" and "Range : "). * Add new line and hyphen only when there are more top level OR than one. --- scripts/kconfig/expr.c | 16 +++++++++++++--- scripts/kconfig/menu.c | 4 ++++ 2 files changed, 17 insertions(+), 3 deletions(-)