2018-02-20 08:21:20

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH] kconfig: clean-up reverse dependency help implementation

This commit splits out the special E_OR handling ('-' instead of '||')
into a dedicated helper expr_print_revdev().

Restore the original expr_print() prior to commit 1ccb27143360
("kconfig: make "Selected by:" and "Implied by:" readable").

This makes sense because:

- We need to chop those expressions only when printing the reverse
dependency, and only when E_OR is encountered

- Otherwise, it should be printed as before, so fall back to
expr_print()

This also improves the behavior; for a single line, it was previously
displayed in the same line as "Selected by", like this:

Selected by: A [=n] && B [=n]

This will be displayed in a new line, consistently:

Selected by:
- A [=n] && B [=n]

Signed-off-by: Masahiro Yamada <[email protected]>
---

scripts/kconfig/expr.c | 36 +++++++++++++++++++++---------------
scripts/kconfig/menu.c | 6 ++----
2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
index d453819..cd3a8f5 100644
--- a/scripts/kconfig/expr.c
+++ b/scripts/kconfig/expr.c
@@ -1179,7 +1179,9 @@ struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2)
return expr_get_leftmost_symbol(ret);
}

-static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken, bool revdep)
+void expr_print(struct expr *e,
+ void (*fn)(void *, struct symbol *, const char *),
+ void *data, int prevtoken)
{
if (!e) {
fn(data, NULL, "y");
@@ -1234,14 +1236,9 @@ 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
- fn(data, NULL, " || ");
- __expr_print(e->right.expr, fn, data, E_OR, revdep);
+ expr_print(e->left.expr, fn, data, E_OR);
+ fn(data, NULL, " || ");
+ expr_print(e->right.expr, fn, data, E_OR);
break;
case E_AND:
expr_print(e->left.expr, fn, data, E_AND);
@@ -1274,11 +1271,6 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con
fn(data, NULL, ")");
}

-void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken)
-{
- __expr_print(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);
@@ -1329,7 +1321,21 @@ void expr_gstr_print(struct expr *e, struct gstr *gs)
* line with a minus. This makes expressions much easier to read.
* Suitable for reverse dependency expressions.
*/
+static void expr_print_revdep(struct expr *e,
+ void (*fn)(void *, struct symbol *, const char *),
+ void *data)
+{
+ if (e->type == E_OR) {
+ expr_print_revdep(e->left.expr, fn, data);
+ expr_print_revdep(e->right.expr, fn, data);
+ } else {
+ fn(data, NULL, " - ");
+ expr_print(e, fn, data, E_NONE);
+ fn(data, NULL, "\n");
+ }
+}
+
void expr_gstr_print_revdep(struct expr *e, struct gstr *gs)
{
- __expr_print(e, expr_print_gstr_helper, gs, E_NONE, true);
+ expr_print_revdep(e, expr_print_gstr_helper, gs);
}
diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 9922285..7e70be3 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -827,16 +827,14 @@ 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_revdep(sym->rev_dep.expr, r);
- str_append(r, "\n");
}

get_symbol_props_str(r, sym, P_IMPLY, _(" Implies: "));
if (sym->implied.expr) {
- str_append(r, _(" Implied by: "));
+ str_append(r, _(" Implied by: \n"));
expr_gstr_print_revdep(sym->implied.expr, r);
- str_append(r, "\n");
}

str_append(r, "\n\n");
--
2.7.4



2018-02-21 11:15:51

by Petr Vorel

[permalink] [raw]
Subject: Re: [PATCH] kconfig: clean-up reverse dependency help implementation

Hi Masahiro,

> This commit splits out the special E_OR handling ('-' instead of '||')
> into a dedicated helper expr_print_revdev().

> Restore the original expr_print() prior to commit 1ccb27143360
> ("kconfig: make "Selected by:" and "Implied by:" readable").

> This makes sense because:

> - We need to chop those expressions only when printing the reverse
> dependency, and only when E_OR is encountered

> - Otherwise, it should be printed as before, so fall back to
> expr_print()

> This also improves the behavior; for a single line, it was previously
> displayed in the same line as "Selected by", like this:

> Selected by: A [=n] && B [=n]

> This will be displayed in a new line, consistently:

> Selected by:
> - A [=n] && B [=n]

> Signed-off-by: Masahiro Yamada <[email protected]>
Reviewed-by: Petr Vorel <[email protected]>
> ---

<snip>

> 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"));
^
I'd remove unnecessary whitespace here ^.

str_append(r, _(" Selected by:\n"));
> expr_gstr_print_revdep(sym->rev_dep.expr, r);
> - str_append(r, "\n");
> }

> get_symbol_props_str(r, sym, P_IMPLY, _(" Implies: "));
> if (sym->implied.expr) {
> - str_append(r, _(" Implied by: "));
> + str_append(r, _(" Implied by: \n"));
The same here.

> expr_gstr_print_revdep(sym->implied.expr, r);
> - str_append(r, "\n");
> }

> str_append(r, "\n\n");


Kind regards,
Petr

2018-02-24 15:19:18

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] kconfig: clean-up reverse dependency help implementation

2018-02-21 20:14 GMT+09:00 Petr Vorel <[email protected]>:
> Hi Masahiro,
>
>> This commit splits out the special E_OR handling ('-' instead of '||')
>> into a dedicated helper expr_print_revdev().
>
>> Restore the original expr_print() prior to commit 1ccb27143360
>> ("kconfig: make "Selected by:" and "Implied by:" readable").
>
>> This makes sense because:
>
>> - We need to chop those expressions only when printing the reverse
>> dependency, and only when E_OR is encountered
>
>> - Otherwise, it should be printed as before, so fall back to
>> expr_print()
>
>> This also improves the behavior; for a single line, it was previously
>> displayed in the same line as "Selected by", like this:
>
>> Selected by: A [=n] && B [=n]
>
>> This will be displayed in a new line, consistently:
>
>> Selected by:
>> - A [=n] && B [=n]
>
>> Signed-off-by: Masahiro Yamada <[email protected]>
> Reviewed-by: Petr Vorel <[email protected]>
>> ---
>
> <snip>
>
>> 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"));
> ^
> I'd remove unnecessary whitespace here ^.


I decided to not touch the text inside _( ... ) in this patch
although I do not think anybody translated " Selected by: ".

I just moved str_append(r, "\n");

Anyway, this line will be removed soon.


Applied to linux-kbuild/kconfig.







> str_append(r, _(" Selected by:\n"));
>> expr_gstr_print_revdep(sym->rev_dep.expr, r);
>> - str_append(r, "\n");
>> }
>
>> get_symbol_props_str(r, sym, P_IMPLY, _(" Implies: "));
>> if (sym->implied.expr) {
>> - str_append(r, _(" Implied by: "));
>> + str_append(r, _(" Implied by: \n"));
> The same here.
>
>> expr_gstr_print_revdep(sym->implied.expr, r);
>> - str_append(r, "\n");
>> }
>
>> str_append(r, "\n\n");
>
>
> Kind regards,
> Petr
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Best Regards
Masahiro Yamada

2018-02-24 16:00:47

by thetruthbeforeus

[permalink] [raw]
Subject: Re: [PATCH] kconfig: clean-up reverse dependency help implementation

Masahiro Yamada: you like anime, don't you.
You like what anime promotes, don't you.
Being a japanese.

On 2018-02-24 15:17, Masahiro Yamada wrote:
> 2018-02-21 20:14 GMT+09:00 Petr Vorel <[email protected]>:
>> Hi Masahiro,
>>
>>> This commit splits out the special E_OR handling ('-' instead of
>>> '||')
>>> into a dedicated helper expr_print_revdev().
>>
>>> Restore the original expr_print() prior to commit 1ccb27143360
>>> ("kconfig: make "Selected by:" and "Implied by:" readable").
>>
>>> This makes sense because:
>>
>>> - We need to chop those expressions only when printing the reverse
>>> dependency, and only when E_OR is encountered
>>
>>> - Otherwise, it should be printed as before, so fall back to
>>> expr_print()
>>
>>> This also improves the behavior; for a single line, it was previously
>>> displayed in the same line as "Selected by", like this:
>>
>>> Selected by: A [=n] && B [=n]
>>
>>> This will be displayed in a new line, consistently:
>>
>>> Selected by:
>>> - A [=n] && B [=n]
>>
>>> Signed-off-by: Masahiro Yamada <[email protected]>
>> Reviewed-by: Petr Vorel <[email protected]>
>>> ---
>>
>> <snip>
>>
>>> 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"));
>> ^
>> I'd remove unnecessary whitespace here ^.
>
>
> I decided to not touch the text inside _( ... ) in this patch
> although I do not think anybody translated " Selected by: ".
>
> I just moved str_append(r, "\n");
>
> Anyway, this line will be removed soon.
>
>
> Applied to linux-kbuild/kconfig.
>
>
>
>
>
>
>
>> str_append(r, _(" Selected by:\n"));
>>> expr_gstr_print_revdep(sym->rev_dep.expr, r);
>>> - str_append(r, "\n");
>>> }
>>
>>> get_symbol_props_str(r, sym, P_IMPLY, _(" Implies: "));
>>> if (sym->implied.expr) {
>>> - str_append(r, _(" Implied by: "));
>>> + str_append(r, _(" Implied by: \n"));
>> The same here.
>>
>>> expr_gstr_print_revdep(sym->implied.expr, r);
>>> - str_append(r, "\n");
>>> }
>>
>>> str_append(r, "\n\n");
>>
>>
>> Kind regards,
>> Petr
>> --
>> To unsubscribe from this list: send the line "unsubscribe
>> linux-kbuild" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html