| Message ID | 20170223131008.13705-1-pvorel@suse.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, > 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. > This also prevents trimming too long lines. > Based on patch from Paul Bolle. > Reported-by: Paul Bolle <pebolle@tiscali.nl> > Cc: Paul Bolle <pebolle@tiscali.nl> > Signed-off-by: Petr Vorel <pvorel@suse.cz> > --- > scripts/kconfig/expr.c | 16 +++++++++++++--- > scripts/kconfig/menu.c | 2 +- > 2 files changed, 14 insertions(+), 4 deletions(-) > 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..3daf7b81637f 100644 > --- a/scripts/kconfig/menu.c > +++ b/scripts/kconfig/menu.c > @@ -675,7 +675,7 @@ 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: ")); > + str_append(r, _(" Selected by: \n - ")); > expr_gstr_print(sym->rev_dep.expr, r); > str_append(r, "\n"); > } Any comment on this patch, please? 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, 2017-04-03 18:35 GMT+09:00 Petr Vorel <pvorel@suse.cz>: > Hi, > >> 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. > >> This also prevents trimming too long lines. > >> Based on patch from Paul Bolle. > >> Reported-by: Paul Bolle <pebolle@tiscali.nl> >> Cc: Paul Bolle <pebolle@tiscali.nl> >> Signed-off-by: Petr Vorel <pvorel@suse.cz> >> --- >> scripts/kconfig/expr.c | 16 +++++++++++++--- >> scripts/kconfig/menu.c | 2 +- >> 2 files changed, 14 insertions(+), 4 deletions(-) > >> 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..3daf7b81637f 100644 >> --- a/scripts/kconfig/menu.c >> +++ b/scripts/kconfig/menu.c >> @@ -675,7 +675,7 @@ 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: ")); >> + str_append(r, _(" Selected by: \n - ")); >> expr_gstr_print(sym->rev_dep.expr, r); >> str_append(r, "\n"); >> } > > Any comment on this patch, please? I noticed you added me in the To: list. Basically I think this patch is a nice idea, but I am not a Kconfig maintainer. Sorry. (I could pick up a really trivial one via kbuild tree somehow, but I want this one to be handled by a person with expertise in the area.) Yann E. MORIN is the Kconfig maintainer, but he has been silent for a few years. I guess Kconfig needs a new maintainer. Please take my comment just as a Kconfig user's point of view. The git-log only mentions "select", but I notice this patch also changes "depends on" format in a bit different way. For example, General setup ---> Kernel compression mode (Gzip) ---> shows its help like this: Depends on: HAVE_KERNEL_GZIP [=y] - HAVE_KERNEL_BZIP2 [=n] - HAVE_KERNEL_LZMA [=y] - HAVE_KERNEL_XZ [=y] - HAVE_KERNEL_LZO [=y] - HAVE_KERNEL_LZ4 [=y] (the first dependency in the same line in the "Depends on")
Hi Masahiro, > Hi Petr, > 2017-04-03 18:35 GMT+09:00 Petr Vorel <pvorel@suse.cz>: > > Hi, > >> 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. > >> This also prevents trimming too long lines. > >> Based on patch from Paul Bolle. <snip> > > Any comment on this patch, please? > I noticed you added me in the To: list. Thank you for your comments. I'm sorry, it took me rather long time to reply. > Basically I think this patch is a nice idea, > but I am not a Kconfig maintainer. Sorry. > (I could pick up a really trivial one via kbuild tree somehow, > but I want this one to be handled by a person with expertise in the area.) > Yann E. MORIN is the Kconfig maintainer, but he has been > silent for a few years. I guess Kconfig needs a new maintainer. Well, Yann is still listed in MAINTAINERS file, even he's inactive (according to the date of his git tree). > Please take my comment just as a Kconfig user's point of view. > The git-log only mentions "select", but I notice this patch > also changes "depends on" format in a bit different way. > For example, > General setup ---> > Kernel compression mode (Gzip) ---> > shows its help like this: > Depends on: HAVE_KERNEL_GZIP [=y] > - HAVE_KERNEL_BZIP2 [=n] > - HAVE_KERNEL_LZMA [=y] > - HAVE_KERNEL_XZ [=y] > - HAVE_KERNEL_LZO [=y] > - HAVE_KERNEL_LZ4 [=y] > (the first dependency in the same line in the "Depends on") You're right, I overlooked it. Do you think it makes sense use it also for "Depends on" section? 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
Petr, All, On 2017-09-20 16:26 +0200, Petr Vorel spake thusly: > > 2017-04-03 18:35 GMT+09:00 Petr Vorel <pvorel@suse.cz>: > > Yann E. MORIN is the Kconfig maintainer, but he has been > > silent for a few years. I guess Kconfig needs a new maintainer. > Well, Yann is still listed in MAINTAINERS file, even he's inactive (according to the date > of his git tree). I already sent a patch relinquishing maintainership a while ago: https://lkml.org/lkml/2017/4/10/1096 Regards, Yann E. MORIN.
Hi Petr, 2017-09-20 23:26 GMT+09:00 Petr Vorel <petr.vorel@gmail.com>: > Hi Masahiro, > >> Hi Petr, > >> 2017-04-03 18:35 GMT+09:00 Petr Vorel <pvorel@suse.cz>: >> > Hi, > >> >> 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. > >> >> This also prevents trimming too long lines. > >> >> Based on patch from Paul Bolle. > <snip> > >> > Any comment on this patch, please? > > >> I noticed you added me in the To: list. > Thank you for your comments. > I'm sorry, it took me rather long time to reply. > >> Basically I think this patch is a nice idea, >> but I am not a Kconfig maintainer. Sorry. >> (I could pick up a really trivial one via kbuild tree somehow, >> but I want this one to be handled by a person with expertise in the area.) > >> Yann E. MORIN is the Kconfig maintainer, but he has been >> silent for a few years. I guess Kconfig needs a new maintainer. > Well, Yann is still listed in MAINTAINERS file, even he's inactive (according to the date > of his git tree). > > >> Please take my comment just as a Kconfig user's point of view. > >> The git-log only mentions "select", but I notice this patch >> also changes "depends on" format in a bit different way. > >> For example, > >> General setup ---> >> Kernel compression mode (Gzip) ---> > > >> shows its help like this: > >> Depends on: HAVE_KERNEL_GZIP [=y] >> - HAVE_KERNEL_BZIP2 [=n] >> - HAVE_KERNEL_LZMA [=y] >> - HAVE_KERNEL_XZ [=y] >> - HAVE_KERNEL_LZO [=y] >> - HAVE_KERNEL_LZ4 [=y] > >> (the first dependency in the same line in the "Depends on") > > You're right, I overlooked it. Do you think it makes sense use it also for "Depends on" > section? Yeah, I guess printing it in a consistent way will be better.
Hi Yann 2017-09-21 1:38 GMT+09:00 Yann E. MORIN <yann.morin.1998@free.fr>: > Petr, All, > > On 2017-09-20 16:26 +0200, Petr Vorel spake thusly: >> > 2017-04-03 18:35 GMT+09:00 Petr Vorel <pvorel@suse.cz>: >> > Yann E. MORIN is the Kconfig maintainer, but he has been >> > silent for a few years. I guess Kconfig needs a new maintainer. >> Well, Yann is still listed in MAINTAINERS file, even he's inactive (according to the date >> of his git tree). > > I already sent a patch relinquishing maintainership a while ago: > https://lkml.org/lkml/2017/4/10/1096 > Can you send it to Linus to pick it up directly?
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..3daf7b81637f 100644 --- a/scripts/kconfig/menu.c +++ b/scripts/kconfig/menu.c @@ -675,7 +675,7 @@ 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: ")); + str_append(r, _(" Selected by: \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. 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. This also prevents trimming too long lines. Based on patch from Paul Bolle. Reported-by: Paul Bolle <pebolle@tiscali.nl> Cc: Paul Bolle <pebolle@tiscali.nl> Signed-off-by: Petr Vorel <pvorel@suse.cz> --- scripts/kconfig/expr.c | 16 +++++++++++++--- scripts/kconfig/menu.c | 2 +- 2 files changed, 14 insertions(+), 4 deletions(-)