diff mbox

kconfig: clean-up reverse dependency help implementation

Message ID 1519114728-1084-1-git-send-email-yamada.masahiro@socionext.com (mailing list archive)
State New, archived
Headers show

Commit Message

Masahiro Yamada Feb. 20, 2018, 8:18 a.m. UTC
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 <yamada.masahiro@socionext.com>
---

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

Comments

Petr Vorel Feb. 21, 2018, 11:14 a.m. UTC | #1
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 <yamada.masahiro@socionext.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
> ---

<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
--
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
Masahiro Yamada Feb. 24, 2018, 3:17 p.m. UTC | #2
2018-02-21 20:14 GMT+09:00 Petr Vorel <petr.vorel@gmail.com>:
> 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 <yamada.masahiro@socionext.com>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>> ---
>
> <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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
thetruthbeforeus@firemail.cc Feb. 24, 2018, 3:58 p.m. UTC | #3
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 <petr.vorel@gmail.com>:
>> 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 <yamada.masahiro@socionext.com>
>> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>>> ---
>> 
>> <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 majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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 mbox

Patch

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");