| Message ID | 20180213005610.10575-1-rosca.eugeniu@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 13, 2018 at 1:56 AM, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote: > From: Eugeniu Rosca <erosca@de.adit-jv.com> > > Commit 1ccb27143360 ("kconfig: make "Selected by:" and "Implied by:" > readable") made an incredible improvement in how reverse dependencies > are perceived by the user, by breaking down the single (often > interminable) expression string into small readable chunks, each of > them displayed on a separate line: > > Selected by: > - A & B > - C & (D || E) > > Unfortunately, what happens with the non-OR (either E_SYMBOL or E_AND) > expressions is that they don't get a dedicated line: > > Selected by: F & G > > That's arguably a bug/misbehavior, but it makes the implementation of > something like below more complicated: > > Selected by: > - [m] F & G /* where (F & G) evaluates to '=m' */ > > Adding '[m]', '[y]' or '[ ]' to the left side of each reverse dependency > is subject of a different commit. The purpose of current commit is to > print the 'Selected by:' and 'Implied by:' expressions on a separate > line _consistently_. > > An example of change contributed by this commit is seen below. > > │ Symbol: NEED_SG_DMA_LENGTH [=y] > │ ... > │ Selected by: IOMMU_DMA [=y] && IOMMU_SUPPORT [=y] > > becomes: > > │ Symbol: NEED_SG_DMA_LENGTH [=y] > │ ... > │ Selected by: > │ - IOMMU_DMA [=y] && IOMMU_SUPPORT [=y] > > This patch has been tested using a custom variant of zconfdump which > prints the reverse dependencies for each config symbol. > > Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com> > Suggested-by: Ulf Magnusson <ulfalizer@gmail.com> > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> > --- > scripts/kconfig/expr.c | 31 ++++++++++++++++++++++--------- > 1 file changed, 22 insertions(+), 9 deletions(-) > > diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c > index d45381986ac7..0704bcdf4c78 100644 > --- a/scripts/kconfig/expr.c > +++ b/scripts/kconfig/expr.c > @@ -1179,6 +1179,16 @@ struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2) > return expr_get_leftmost_symbol(ret); > } > > +static void > +expr_print_newline(struct expr *e, > + void (*fn)(void *, struct symbol *, const char *), > + void *data, > + int prevtoken) > +{ > + fn(data, NULL, "\n - "); > + expr_print(e, fn, data, prevtoken); > +} > + > static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken, bool revdep) > { > if (!e) { > @@ -1191,7 +1201,10 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con > switch (e->type) { > case E_SYMBOL: > if (e->left.sym->name) > - fn(data, e->left.sym, e->left.sym->name); > + if (revdep) > + expr_print_newline(e, fn, data, E_OR); > + else > + fn(data, e->left.sym, e->left.sym->name); > else > fn(data, NULL, "<choice>"); > break; > @@ -1234,19 +1247,19 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con > fn(data, e->right.sym, e->right.sym->name); > break; > case E_OR: > - if (revdep && e->left.expr->type != E_OR) > - fn(data, NULL, "\n - "); > __expr_print(e->left.expr, fn, data, E_OR, revdep); > - if (revdep) > - fn(data, NULL, "\n - "); > - else > + if (!revdep) > fn(data, NULL, " || "); > __expr_print(e->right.expr, fn, data, E_OR, revdep); > break; > case E_AND: > - expr_print(e->left.expr, fn, data, E_AND); > - fn(data, NULL, " && "); > - expr_print(e->right.expr, fn, data, E_AND); > + if (revdep) { > + expr_print_newline(e, fn, data, E_OR); > + } else { > + expr_print(e->left.expr, fn, data, E_AND); > + fn(data, NULL, " && "); > + expr_print(e->right.expr, fn, data, E_AND); > + } > break; > case E_LIST: > fn(data, e->right.sym, e->right.sym->name); > -- > 2.16.1 > Reviewed-by: Ulf Magnusson <ulfalizer@gmail.com> Cheers, Ulf -- 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
Hi Eugeniu, > On Tue, Feb 13, 2018 at 1:56 AM, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote: > > From: Eugeniu Rosca <erosca@de.adit-jv.com> > > Commit 1ccb27143360 ("kconfig: make "Selected by:" and "Implied by:" > > readable") made an incredible improvement in how reverse dependencies > > are perceived by the user, by breaking down the single (often > > interminable) expression string into small readable chunks, each of > > them displayed on a separate line: > > Selected by: > > - A & B > > - C & (D || E) > > Unfortunately, what happens with the non-OR (either E_SYMBOL or E_AND) > > expressions is that they don't get a dedicated line: > > Selected by: F & G I deliberately choose it :-). It didn't make much sense for me to add new line for just one line. I thought someone wouldn't like it for the inconsistency :-). 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
Hi Petr, On Tue, Feb 13, 2018 at 07:40:30AM +0100, Petr Vorel wrote: > Hi Eugeniu, > > > On Tue, Feb 13, 2018 at 1:56 AM, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote: > > > From: Eugeniu Rosca <erosca@de.adit-jv.com> > > > > Commit 1ccb27143360 ("kconfig: make "Selected by:" and "Implied by:" > > > readable") made an incredible improvement in how reverse dependencies > > > are perceived by the user, by breaking down the single (often > > > interminable) expression string into small readable chunks, each of > > > them displayed on a separate line: > > > > Selected by: > > > - A & B > > > - C & (D || E) > > > > Unfortunately, what happens with the non-OR (either E_SYMBOL or E_AND) > > > expressions is that they don't get a dedicated line: > > > > Selected by: F & G > I deliberately choose it :-). It didn't make much sense for me to add new line for just > one line. I thought someone wouldn't like it for the inconsistency :-). I have to say I do like how `Selected by: F && G` looks on a single row. However, when the expression is prefixed by its value (i.e. `Selected by: [m] F && G`), imho it doesn't look as crisp and clear as we would like it to (feel free to disagree here). Fwiw, this patch doesn't see itself as a fix, but rather prepares the ground for the next commit implementing prefixing reverse dependencies by their expression value. > > Kind regards, > Petr Thanks! Eugeniu. -- 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 d45381986ac7..0704bcdf4c78 100644 --- a/scripts/kconfig/expr.c +++ b/scripts/kconfig/expr.c @@ -1179,6 +1179,16 @@ struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2) return expr_get_leftmost_symbol(ret); } +static void +expr_print_newline(struct expr *e, + void (*fn)(void *, struct symbol *, const char *), + void *data, + int prevtoken) +{ + fn(data, NULL, "\n - "); + expr_print(e, fn, data, prevtoken); +} + static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken, bool revdep) { if (!e) { @@ -1191,7 +1201,10 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con switch (e->type) { case E_SYMBOL: if (e->left.sym->name) - fn(data, e->left.sym, e->left.sym->name); + if (revdep) + expr_print_newline(e, fn, data, E_OR); + else + fn(data, e->left.sym, e->left.sym->name); else fn(data, NULL, "<choice>"); break; @@ -1234,19 +1247,19 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con fn(data, e->right.sym, e->right.sym->name); break; case E_OR: - if (revdep && e->left.expr->type != E_OR) - fn(data, NULL, "\n - "); __expr_print(e->left.expr, fn, data, E_OR, revdep); - if (revdep) - fn(data, NULL, "\n - "); - else + if (!revdep) fn(data, NULL, " || "); __expr_print(e->right.expr, fn, data, E_OR, revdep); break; case E_AND: - expr_print(e->left.expr, fn, data, E_AND); - fn(data, NULL, " && "); - expr_print(e->right.expr, fn, data, E_AND); + if (revdep) { + expr_print_newline(e, fn, data, E_OR); + } else { + expr_print(e->left.expr, fn, data, E_AND); + fn(data, NULL, " && "); + expr_print(e->right.expr, fn, data, E_AND); + } break; case E_LIST: fn(data, e->right.sym, e->right.sym->name);