[PATCH v2] menuconfig: distinguish between selected-by-another options and comments

[PATCH v2] menuconfig: distinguish between selected-by-another options and comments

[Matej Laitl]

menuconfig currently represents options implied by another option ('select'
directive in Kconfig) by prefixing them with '---'. Unfortunately the same
notation is used for comments.

This patch changes notation of selected-by-another items by introducing 2 new
representations for implied options:
{*} or {M} for options selected by another modularized one, thus builtin or
module capable,
-*- or -M- for options that cannot be at the moment changed by user.

The idea is to represent actual capability of the option by braces (dashes)
around and to always report actual state by * or M inside.

Signed-off-by: Matěj Laitl <strohel@gmail.com>
---
Changes since v1:
* introduce sym_get_minimal_value(), so that access to struct symbol is 
abstracted.
* change also menuconfig's window header text to reflect the change. I'm still 
not sure if the wording is optimal.

v1 was acked by Randy Dunlap, I'm not sure if it's appropriate to keep 
Acked-by when patch changes. (teach me please)

 scripts/kconfig/lkc_proto.h |    1 +
 scripts/kconfig/mconf.c     |   21 +++++++++++++--------
 scripts/kconfig/symbol.c    |    5 +++++
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h
index 4d09f6d..98642bc 100644
--- a/scripts/kconfig/lkc_proto.h
+++ b/scripts/kconfig/lkc_proto.h
@@ -34,6 +34,7 @@ P(sym_string_valid,bool,(struct symbol *sym, const char 
*newval));
 P(sym_string_within_range,bool,(struct symbol *sym, const char *str));
 P(sym_set_string_value,bool,(struct symbol *sym, const char *newval));
 P(sym_is_changable,bool,(struct symbol *sym));
+P(sym_get_minimal_value,tristate,(struct symbol *sym));
 P(sym_get_choice_prop,struct property *,(struct symbol *sym));
 P(sym_get_default_prop,struct property *,(struct symbol *sym));
 P(sym_get_string_value,const char *,(struct symbol *sym));
diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
index bc5854e..77ab552 100644
--- a/scripts/kconfig/mconf.c
+++ b/scripts/kconfig/mconf.c
@@ -35,9 +35,13 @@ static const char mconf_readme[] = N_(
 "kernel parameters which are not really features, but must be\n"
 "entered in as decimal or hexadecimal numbers or possibly text.\n"
 "\n"
-"Menu items beginning with [*], <M> or [ ] represent features\n"
-"configured to be built in, modularized or removed respectively.\n"
-"Pointed brackets <> represent module capable features.\n"
+"Menu items beginning with following braces represent features that\n"
+"  [ ] can be built in or removed\n"
+"  < > can be built in, modularized or removed\n"
+"  { } can be built in or modularized (selected by other feature)\n"
+"  - - are selected by other feature,\n"
+"while *, M or whitespace inside braces means to build in, build as\n"
+"a module or to exclude the feature respectively.\n"
 "\n"
 "To change any of these features, highlight it with the cursor\n"
 "keys and press <Y> to build it in, <M> to make it a module or\n"
@@ -178,9 +182,9 @@ menu_instructions[] = N_(
 	"Arrow keys navigate the menu.  "
 	"<Enter> selects submenus --->.  "
 	"Highlighted letters are hotkeys.  "
-	"Pressing <Y> includes, <N> excludes, <M> modularizes features.  "
+	"Pressing <Y> includes (*), <M> modularizes (M), <N> excludes features 
( ).  "
 	"Press <Esc><Esc> to exit, <?> for Help, </> for Search.  "
-	"Legend: [*] built-in  [ ] excluded  <M> module  < > module capable"),
+	"Legend: [*] built-in, < > module capable, { } implied, -*- forced."),
 radiolist_instructions[] = N_(
 	"Use the arrow keys to navigate this window or "
 	"press the hotkey of the item you wish to select "
@@ -560,7 +564,7 @@ static void build_conf(struct menu *menu)
 				if (sym_is_changable(sym))
 					item_make("[%c]", val == no ? ' ' : '*');
 				else
-					item_make("---");
+					item_make("-%c-", val == no ? ' ' : '*');
 				item_set_tag('t');
 				item_set_data(menu);
 				break;
@@ -571,9 +575,10 @@ static void build_conf(struct menu *menu)
 				default:  ch = ' '; break;
 				}
 				if (sym_is_changable(sym))
-					item_make("<%c>", ch);
+					item_make((sym_get_minimal_value(sym) == mod) ?
+					          "{%c}" : "<%c>", ch);
 				else
-					item_make("---");
+					item_make("-%c-", ch);
 				item_set_tag('t');
 				item_set_data(menu);
 				break;
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index c35dcc5..b9832d0 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -643,6 +643,11 @@ bool sym_is_changable(struct symbol *sym)
 	return sym->visible > sym->rev_dep.tri;
 }
 
+tristate sym_get_minimal_value(struct symbol *sym)
+{
+	return sym->rev_dep.tri;
+}
+
 struct symbol *sym_lookup(const char *name, int isconst)
 {
 	struct symbol *symbol;
-- 
1.5.1.6

Re: [PATCH v2] menuconfig: distinguish between selected-by-another options and comments

[Matej Laitl]

On Saturday 15 of September 2007 20:04:10 Matej Laitl wrote:
> menuconfig currently represents options implied by another option ('select'
> directive in Kconfig) by prefixing them with '---'. Unfortunately the same
> notation is used for comments.
>
> (...)

Oh please reply to my address strohel@gmail.com. I mismatched Reply-To and 
In-Reply-To e-mail headers when trying to stick the message to original 
thread.

Sorry for the noise and inconvenience I caused.

Re: [PATCH v2] menuconfig: distinguish between selected-by-another options and comments

[Jan Engelhardt]

On Sep 15 2007 20:04, Matej Laitl wrote:
>
>This patch changes notation of selected-by-another items by introducing 2 new
>representations for implied options:
>{*} or {M} for options selected by another modularized one, thus builtin or
>module capable,
>-*- or -M- for options that cannot be at the moment changed by user.

This gets rid of the ambiguous ---, does not it? (If so, nice!)

Re: [PATCH v2] menuconfig: distinguish between selected-by-another options and comments

[Matej Laitl]

On Saturday 15 of September 2007 20:38:14 you wrote:
> On Sep 15 2007 20:04, Matej Laitl wrote:
> >This patch changes notation of selected-by-another items by introducing 2
> > new representations for implied options:
> >{*} or {M} for options selected by another modularized one, thus builtin
> > or module capable,
> >-*- or -M- for options that cannot be at the moment changed by user.
>
> This gets rid of the ambiguous ---, does not it? (If so, nice!)

Definately. You may try it, the patch is against current Linus's HEAD, but may 
apply cleanly also against 2.6.22 as none of the files changed since. (AFAIK, 
too lazy to check it)

Regards,
         Matej

Re: [PATCH v2] menuconfig: distinguish between selected-by-another options and comments

[Randy Dunlap]

On Sat, 15 Sep 2007 20:04:10 +0200 Matej Laitl wrote:

> menuconfig currently represents options implied by another option ('select'
> directive in Kconfig) by prefixing them with '---'. Unfortunately the same
> notation is used for comments.
> 
> This patch changes notation of selected-by-another items by introducing 2 new
> representations for implied options:
> {*} or {M} for options selected by another modularized one, thus builtin or
> module capable,
> -*- or -M- for options that cannot be at the moment changed by user.
> 
> The idea is to represent actual capability of the option by braces (dashes)
> around and to always report actual state by * or M inside.
> 
> Signed-off-by: Matěj Laitl <strohel@gmail.com>
> ---
> Changes since v1:
> * introduce sym_get_minimal_value(), so that access to struct symbol is 
> abstracted.
> * change also menuconfig's window header text to reflect the change. I'm still 
> not sure if the wording is optimal.

I don't see any problems with the wording other than I'm not very
strongly in favor of the word "implied".  I don't have a better
word to use other than "selected", but that's just an implementation
detail, not especially user friendly.


> v1 was acked by Randy Dunlap, I'm not sure if it's appropriate to keep 
> Acked-by when patch changes. (teach me please)

Good question.  I think you did it correctly.

Acked-by: Randy Dunlap <randy.dunlap@oracle.com>

>  scripts/kconfig/lkc_proto.h |    1 +
>  scripts/kconfig/mconf.c     |   21 +++++++++++++--------
>  scripts/kconfig/symbol.c    |    5 +++++
>  3 files changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h
> index 4d09f6d..98642bc 100644
> --- a/scripts/kconfig/lkc_proto.h
> +++ b/scripts/kconfig/lkc_proto.h
> @@ -34,6 +34,7 @@ P(sym_string_valid,bool,(struct symbol *sym, const char 
> *newval));
>  P(sym_string_within_range,bool,(struct symbol *sym, const char *str));
>  P(sym_set_string_value,bool,(struct symbol *sym, const char *newval));
>  P(sym_is_changable,bool,(struct symbol *sym));
> +P(sym_get_minimal_value,tristate,(struct symbol *sym));
>  P(sym_get_choice_prop,struct property *,(struct symbol *sym));
>  P(sym_get_default_prop,struct property *,(struct symbol *sym));
>  P(sym_get_string_value,const char *,(struct symbol *sym));
> diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
> index bc5854e..77ab552 100644
> --- a/scripts/kconfig/mconf.c
> +++ b/scripts/kconfig/mconf.c
> @@ -35,9 +35,13 @@ static const char mconf_readme[] = N_(
>  "kernel parameters which are not really features, but must be\n"
>  "entered in as decimal or hexadecimal numbers or possibly text.\n"
>  "\n"
> -"Menu items beginning with [*], <M> or [ ] represent features\n"
> -"configured to be built in, modularized or removed respectively.\n"
> -"Pointed brackets <> represent module capable features.\n"
> +"Menu items beginning with following braces represent features that\n"
> +"  [ ] can be built in or removed\n"
> +"  < > can be built in, modularized or removed\n"
> +"  { } can be built in or modularized (selected by other feature)\n"
> +"  - - are selected by other feature,\n"
> +"while *, M or whitespace inside braces means to build in, build as\n"
> +"a module or to exclude the feature respectively.\n"
>  "\n"
>  "To change any of these features, highlight it with the cursor\n"
>  "keys and press <Y> to build it in, <M> to make it a module or\n"
> @@ -178,9 +182,9 @@ menu_instructions[] = N_(
>  	"Arrow keys navigate the menu.  "
>  	"<Enter> selects submenus --->.  "
>  	"Highlighted letters are hotkeys.  "
> -	"Pressing <Y> includes, <N> excludes, <M> modularizes features.  "
> +	"Pressing <Y> includes (*), <M> modularizes (M), <N> excludes features 
> ( ).  "
>  	"Press <Esc><Esc> to exit, <?> for Help, </> for Search.  "
> -	"Legend: [*] built-in  [ ] excluded  <M> module  < > module capable"),
> +	"Legend: [*] built-in, < > module capable, { } implied, -*- forced."),
>  radiolist_instructions[] = N_(
>  	"Use the arrow keys to navigate this window or "
>  	"press the hotkey of the item you wish to select "
> @@ -560,7 +564,7 @@ static void build_conf(struct menu *menu)
>  				if (sym_is_changable(sym))
>  					item_make("[%c]", val == no ? ' ' : '*');
>  				else
> -					item_make("---");
> +					item_make("-%c-", val == no ? ' ' : '*');
>  				item_set_tag('t');
>  				item_set_data(menu);
>  				break;
> @@ -571,9 +575,10 @@ static void build_conf(struct menu *menu)
>  				default:  ch = ' '; break;
>  				}
>  				if (sym_is_changable(sym))
> -					item_make("<%c>", ch);
> +					item_make((sym_get_minimal_value(sym) == mod) ?
> +					          "{%c}" : "<%c>", ch);
>  				else
> -					item_make("---");
> +					item_make("-%c-", ch);
>  				item_set_tag('t');
>  				item_set_data(menu);
>  				break;
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index c35dcc5..b9832d0 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -643,6 +643,11 @@ bool sym_is_changable(struct symbol *sym)
>  	return sym->visible > sym->rev_dep.tri;
>  }
>  
> +tristate sym_get_minimal_value(struct symbol *sym)
> +{
> +	return sym->rev_dep.tri;
> +}
> +
>  struct symbol *sym_lookup(const char *name, int isconst)
>  {
>  	struct symbol *symbol;
> -- 

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH v2] menuconfig: distinguish between selected-by-another options and comments

[Sam Ravnborg]

On Sat, Sep 15, 2007 at 08:04:10PM +0200, Matej Laitl wrote:
> menuconfig currently represents options implied by another option ('select'
> directive in Kconfig) by prefixing them with '---'. Unfortunately the same
> notation is used for comments.
> 
> This patch changes notation of selected-by-another items by introducing 2 new
> representations for implied options:
> {*} or {M} for options selected by another modularized one, thus builtin or
> module capable,
> -*- or -M- for options that cannot be at the moment changed by user.

With this patch we can now distingush between comments and menulines
that the user cannot cahnge.
As one of the major issues with kconfig is the evil select handling
it would have been much preferred to distingush between lines
that are selected-by and lines that cannot be changed for other
reasons.
The block menu is a good example where we do not sue select. It is marked
'---' because it is default y if EMBEDDED is not selected.

So the intention is clear and your changed interface (sym_get_min_val())
look OK.
But can you take a look at distingushing between non-selectable options
due to dependency issues and seleted-by symbols.

	Sam

Re: [PATCH v2] menuconfig: distinguish between selected-by-another options and comments

[Roman Zippel]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 940 bytes --]

Hi,

On Sat, 15 Sep 2007, Matej Laitl wrote:

> menuconfig currently represents options implied by another option ('select'
> directive in Kconfig) by prefixing them with '---'. Unfortunately the same
> notation is used for comments.
> 
> This patch changes notation of selected-by-another items by introducing 2 new
> representations for implied options:
> {*} or {M} for options selected by another modularized one, thus builtin or
> module capable,
> -*- or -M- for options that cannot be at the moment changed by user.

I shortly considered doing something like this, but initially I wanted to
keep it simple. It's a compromise between overloading the user interface
with information and keeping it simple. That's the only concern I have
with it, but if everyone else likes I don't mind either. :)

> Signed-off-by: Matěj Laitl <strohel@gmail.com>

Signed-off-by: Roman Zippel <zippel@linux-m68k.org>

bye, Roman

Re: [PATCH v2] menuconfig: distinguish between selected-by-another options and comments

[Roman Zippel]

Hi,

On Sun, 16 Sep 2007, Sam Ravnborg wrote:

> But can you take a look at distingushing between non-selectable options
> due to dependency issues and seleted-by symbols.

Do you have an example in mind? If a symbol is not changable, but still 
visible, a select is usually involved.

bye, Roman

[PATCH v3] menuconfig: distinguish between selected-by-another options and comments

[Matej Laitl]

menuconfig currently represents options implied by another option ('select'
directive in Kconfig) by prefixing them with '---'. Unfortunately the same
notation is used for comments.

This patch changes notation of selected-by-another items by introducing new
representations for implied options:
{*} or {M} for module capable selected-by features
(*) for module incapable selected-by features

Additionally introduce new notation for features unchangeable due to unsatisfied
dependency of prompt field: "-*-" or "- -". (See CONFIG_BLOCK)

Add help text reporting why feature is unchangable.

Signed-off-by: Matěj Laitl <strohel@gmail.com>
---
Hi Sam, Randy, Roman,
I'm presenting third version of the patch, which should address all mentioned
issues with the patch.
I'm not sure if the text in symbol's help is necessary, feel free to exclude
it.

Changes since v2:
* introduce sym_get_maximal_value()
* make selected-by another features distinguishable from features with no 
active prompt, thus unchangeable.
* add text to each unchangeable symbol's help explaining why it's so.

Changes since v1:
* introduce sym_get_minimal_value(), so that access to struct symbol is 
abstracted.
* change also menuconfig's window header text to reflect the change. I'm still 
not sure if the wording is optimal.

 scripts/kconfig/lkc_proto.h |    2 +
 scripts/kconfig/mconf.c     |   55 +++++++++++++++++++++++++++++--------------
 scripts/kconfig/symbol.c    |   10 ++++++++
 3 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h
index 4d09f6d..c2e3461 100644
--- a/scripts/kconfig/lkc_proto.h
+++ b/scripts/kconfig/lkc_proto.h
@@ -34,6 +34,8 @@ P(sym_string_valid,bool,(struct symbol *sym, const char *newval));
 P(sym_string_within_range,bool,(struct symbol *sym, const char *str));
 P(sym_set_string_value,bool,(struct symbol *sym, const char *newval));
 P(sym_is_changable,bool,(struct symbol *sym));
+P(sym_get_minimal_value,tristate,(struct symbol *sym));
+P(sym_get_maximal_value,tristate,(struct symbol *sym));
 P(sym_get_choice_prop,struct property *,(struct symbol *sym));
 P(sym_get_default_prop,struct property *,(struct symbol *sym));
 P(sym_get_string_value,const char *,(struct symbol *sym));
diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
index bc5854e..8f8992e 100644
--- a/scripts/kconfig/mconf.c
+++ b/scripts/kconfig/mconf.c
@@ -35,9 +35,19 @@ static const char mconf_readme[] = N_(
 "kernel parameters which are not really features, but must be\n"
 "entered in as decimal or hexadecimal numbers or possibly text.\n"
 "\n"
-"Menu items beginning with [*], <M> or [ ] represent features\n"
-"configured to be built in, modularized or removed respectively.\n"
-"Pointed brackets <> represent module capable features.\n"
+"Menu items beginning with following braces represent features that\n"
+"  [ ] can be built in or excluded;\n"
+"  < > can be built in, modularized or excluded.\n"
+"Some options may 'select' another options, selected options cannot\n"
+"be excluded from building. Selected features are indicated as follows\n"
+"  ( ) module incapable selected features\n"
+"  { } module capable selected features.\n"
+"Character inside braces represent actual state, where *, M or\n"
+"whitespace means to build in, build as a module or to exclude the\n"
+"feature respectively.\n"
+"\n"
+"Items starting with - -, -M- or -*- cannot changed by the user\n"
+"because they have no active prompt (unsatisfied dependency).\n"
 "\n"
 "To change any of these features, highlight it with the cursor\n"
 "keys and press <Y> to build it in, <M> to make it a module or\n"
@@ -178,9 +188,9 @@ menu_instructions[] = N_(
 	"Arrow keys navigate the menu.  "
 	"<Enter> selects submenus --->.  "
 	"Highlighted letters are hotkeys.  "
-	"Pressing <Y> includes, <N> excludes, <M> modularizes features.  "
+	"Pressing <Y> includes (*), <M> modularizes (M), <N> excludes features ( ).  "
 	"Press <Esc><Esc> to exit, <?> for Help, </> for Search.  "
-	"Legend: [*] built-in  [ ] excluded  <M> module  < > module capable"),
+	"Legend: [*], (*) built-in;  < >, { } module capable."),
 radiolist_instructions[] = N_(
 	"Use the arrow keys to navigate this window or "
 	"press the hotkey of the item you wish to select "
@@ -359,6 +369,11 @@ static void get_symbol_str(struct gstr *r, struct symbol *sym)
 
 	str_printf(r, "Symbol: %s [=%s]\n", sym->name,
 	                               sym_get_string_value(sym));
+	if (sym_get_minimal_value(sym) != no)
+		str_printf(r, "Enforced value: %s (see Selected by:)\n",
+		              sym_get_minimal_value(sym) == mod ? "[m] or [y]" : "[y]");
+	if (sym_get_maximal_value(sym) == no)
+		str_append(r, "None of the prompts active, default value assigned\n");
 	for_all_prompts(sym, prop)
 		get_prompt_str(r, prop);
 	hit = false;
@@ -542,14 +557,19 @@ static void build_conf(struct menu *menu)
 			return;
 		}
 	} else {
+		val = sym_get_tristate_value(sym);
+		switch (val) {
+		case yes: ch = '*'; break;
+		case mod: ch = 'M'; break;
+		default:  ch = ' '; break;
+		}
 		if (menu == current_menu) {
-			item_make("---%*c%s", indent + 1, ' ', menu_get_prompt(menu));
+			item_make("-%c-%*c%s", ch, indent + 1, ' ', menu_get_prompt(menu));
 			item_set_tag(':');
 			item_set_data(menu);
 			goto conf_childs;
 		}
 		child_count++;
-		val = sym_get_tristate_value(sym);
 		if (sym_is_choice_value(sym) && val == yes) {
 			item_make("   ");
 			item_set_tag(':');
@@ -557,23 +577,22 @@ static void build_conf(struct menu *menu)
 		} else {
 			switch (type) {
 			case S_BOOLEAN:
-				if (sym_is_changable(sym))
-					item_make("[%c]", val == no ? ' ' : '*');
+				if (sym_get_maximal_value(sym) == no) /* no prompt visible */
+					item_make("-%c-", ch);
+				else if (sym_get_minimal_value(sym) != no) /* selected-by */
+					item_make("(%c)", ch);
 				else
-					item_make("---");
+					item_make("[%c]", ch);
 				item_set_tag('t');
 				item_set_data(menu);
 				break;
 			case S_TRISTATE:
-				switch (val) {
-				case yes: ch = '*'; break;
-				case mod: ch = 'M'; break;
-				default:  ch = ' '; break;
-				}
-				if (sym_is_changable(sym))
-					item_make("<%c>", ch);
+				if (sym_get_maximal_value(sym) == no) /* no prompt visible */
+					item_make("-%c-", ch);
+				else if (sym_get_minimal_value(sym) != no) /* selected-by */
+					item_make("{%c}", ch);
 				else
-					item_make("---");
+					item_make("<%c>", ch);
 				item_set_tag('t');
 				item_set_data(menu);
 				break;
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index c35dcc5..8fe1d72 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -643,6 +643,16 @@ bool sym_is_changable(struct symbol *sym)
 	return sym->visible > sym->rev_dep.tri;
 }
 
+tristate sym_get_minimal_value(struct symbol *sym)
+{
+	return sym->rev_dep.tri;
+}
+
+tristate sym_get_maximal_value(struct symbol *sym)
+{
+	return sym->visible;
+}
+
 struct symbol *sym_lookup(const char *name, int isconst)
 {
 	struct symbol *symbol;
-- 
1.5.1.6

Re: [PATCH v3] menuconfig: distinguish between selected-by-another options and comments

[Roman Zippel]

Hi,

On Sun, 16 Sep 2007, Matej Laitl wrote:

> +tristate sym_get_minimal_value(struct symbol *sym)
> +{
> +	return sym->rev_dep.tri;
> +}
> +
> +tristate sym_get_maximal_value(struct symbol *sym)
> +{
> +	return sym->visible;
> +}
> +

Right now I prefer the previous version. This maximum value is overridden 
by the minimum value, so I wouldn't like it to be exported like this.

I would really like to see an example, where the new changes make a 
difference.

bye, Roman

Re: [PATCH v2] menuconfig: distinguish between selected-by-another options and comments

[Sam Ravnborg]

On Sun, Sep 16, 2007 at 07:10:06PM +0200, Roman Zippel wrote:
> Hi,
> 
> On Sun, 16 Sep 2007, Sam Ravnborg wrote:
> 
> > But can you take a look at distingushing between non-selectable options
> > due to dependency issues and seleted-by symbols.
> 
> Do you have an example in mind? If a symbol is not changable, but still 
> visible, a select is usually involved.

On i86_64 (but I think the arch does not matter).
make defconfig
make menuconfig

At top-level menu I see:
--- Enable the block layer  --->

In block/Kconfig we have:
menuconfig BLOCK
       bool "Enable the block layer" if EMBEDDED
       default y

If EMBEDDED == n then we see the above.
And this was my first experience with this patch - and it
took me some thoughts to realise it was the "if EMBEDDED" part
that made it look like -*-

So maybe not something we see in many places but it is
there in the top-level menu.

	Sam

Re: [PATCH v3] menuconfig: distinguish between selected-by-another options and comments

[Matej Laitl]

On Sunday 16 of September 2007 19:59:56 Roman Zippel wrote:
> Right now I prefer the previous version.

The v2 was maybe more intuitive, but had at least one flaw, where it claimed
the option was selected by another, while it was in fact only made
unchangeable by 'bool "Enable block layer" if EMBEDDED', defaulting to y.

> This maximum value is overridden  
> by the minimum value, so I wouldn't like it to be exported like this.

Where exactly does this happen? There are cases when maximum < minimum, for
example when you <M> THINKPAD_ACPI, then <N> BACKLIGHT_LCD_SUPPORT. After that,
BACKLIGHT_CLASS_DEVICE has minimum of 1 (selected by THINKPAD_ACPI) and maximum
0 (depends on BACKLIGHT_LCD_SUPPORT, which is n)

The function names are maybe suboptimal, I agree.

> I would really like to see an example, where the new changes make a 
> difference.

Try saying <M> to MAC80211, wireless extensions were -*- with v2, now are (*) with v3.
Also the symbols ( ), { } are changed and more well-defined in v3 IMHO.

Regards,
         Matěj

Re: [PATCH v2] menuconfig: distinguish between selected-by-another options and comments

[Sam Ravnborg]

On Sat, Sep 15, 2007 at 08:04:10PM +0200, Matej Laitl wrote:
> menuconfig currently represents options implied by another option ('select'
> directive in Kconfig) by prefixing them with '---'. Unfortunately the same
> notation is used for comments.
This is easy to fix - example below.

Now a comment looks like this

   *** This is a comment ***

Notice the three leading spaces which is where '---' was before.

	Sam

diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
index bc5854e..2ee12a7 100644
--- a/scripts/kconfig/mconf.c
+++ b/scripts/kconfig/mconf.c
@@ -481,6 +481,14 @@ static void build_conf(struct menu *menu)
 				if (single_menu_mode && menu->data)
 					goto conf_childs;
 				return;
+			case P_COMMENT:
+				if (prompt) {
+					child_count++;
+					item_make("   %*c*** %s ***", indent + 1, ' ', prompt);
+					item_set_tag(':');
+					item_set_data(menu);
+				}
+				break;
 			default:
 				if (prompt) {
 					child_count++;

Re: [PATCH v2] menuconfig: distinguish between selected-by-another options and comments

[Matej Laitl]

On Sunday 16 of September 2007 20:41:10 Sam Ravnborg wrote:
> On Sat, Sep 15, 2007 at 08:04:10PM +0200, Matej Laitl wrote:
> > menuconfig currently represents options implied by another option ('select'
> > directive in Kconfig) by prefixing them with '---'. Unfortunately the same
> > notation is used for comments.
> This is easy to fix - example below.
> 
> Now a comment looks like this
> 
>    *** This is a comment ***
> 
> Notice the three leading spaces which is where '---' was before.
> 
> 	Sam

Definitely this is the best solution for the comment ambiguity.

So my patch turned out to solve only these questions:
"What does the '--- Wireless extensions' mean? Included or excluded?"
"Why I cannot disable '<M> Option XXX'?"

Dunno if it is still worth merging. (but even if not, it at least made you
solve the comments problem) :)

Matěj.

Re: [PATCH v3] menuconfig: distinguish between selected-by-another options and comments

[Roman Zippel]

Hi,

On Sun, 16 Sep 2007, Matej Laitl wrote:

> The v2 was maybe more intuitive, but had at least one flaw, where it claimed
> the option was selected by another, while it was in fact only made
> unchangeable by 'bool "Enable block layer" if EMBEDDED', defaulting to y.

The point is that I'm getting more concerned about overloading the 
interface with nontrivial information.
Another direction to consider would be to add this information to the help 
text, e.g. choose one syntax for nonchangable symbols and then the user 
can press help to find more detailed information.

> > This maximum value is overridden  
> > by the minimum value, so I wouldn't like it to be exported like this.
> 
> Where exactly does this happen?

sym_tristate_within_range() checks whether the new value is ok and 
sym_calc_value() uses rev_dep to override the value when necessary.

> There are cases when maximum < minimum, for
> example when you <M> THINKPAD_ACPI, then <N> BACKLIGHT_LCD_SUPPORT. After that,
> BACKLIGHT_CLASS_DEVICE has minimum of 1 (selected by THINKPAD_ACPI) and maximum
> 0 (depends on BACKLIGHT_LCD_SUPPORT, which is n)

The actual maximum value is then 1.

> The function names are maybe suboptimal, I agree.

The variable name is already correct, it's the visibility value of a 
symbol not its maximum. In the case of the "if EMBEDDED" then individual 
menu entries can still be visible, if any child entry is visible (see 
menu_is_visible()). 

bye, Roman

Re: [PATCH v2] menuconfig: distinguish between selected-by-another options and comments

[Roman Zippel]

Hi,

On Sun, 16 Sep 2007, Sam Ravnborg wrote:

> > On Sun, 16 Sep 2007, Sam Ravnborg wrote:
> > 
> > > But can you take a look at distingushing between non-selectable options
> > > due to dependency issues and seleted-by symbols.
> > 
> > Do you have an example in mind? If a symbol is not changable, but still 
> > visible, a select is usually involved.
> 
> On i86_64 (but I think the arch does not matter).
> make defconfig
> make menuconfig
> 
> At top-level menu I see:
> --- Enable the block layer  --->
> 
> In block/Kconfig we have:
> menuconfig BLOCK
>        bool "Enable the block layer" if EMBEDDED
>        default y
> 
> If EMBEDDED == n then we see the above.
> And this was my first experience with this patch - and it
> took me some thoughts to realise it was the "if EMBEDDED" part
> that made it look like -*-

Indeed, I forgot about this case. Here it's actually the menu entry of a 
symbol that is forced to be visible, because a child entry is visible.

bye, Roman

Re: [PATCH v3] menuconfig: distinguish between selected-by-another options and comments

[Matej Laitl]

On Sunday 16 of September 2007 21:36:54 Roman Zippel wrote:
> On Sun, 16 Sep 2007, Matej Laitl wrote:
> > The v2 was maybe more intuitive, but had at least one flaw, where it claimed
> > the option was selected by another, while it was in fact only made
> > unchangeable by 'bool "Enable block layer" if EMBEDDED', defaulting to y.
> 
> The point is that I'm getting more concerned about overloading the 
> interface with nontrivial information.
> Another direction to consider would be to add this information to the help 
> text, e.g. choose one syntax for nonchangable symbols and then the user 
> can press help to find more detailed information.

If I understand clearly, something similar is already in v3 (hunk took from
in-progress v4):
@@ -359,6 +369,11 @@ static void get_symbol_str(struct gstr *r, struct symbol *sym)
 
        str_printf(r, "Symbol: %s [=%s]\n", sym->name,
                                       sym_get_string_value(sym));
+       if (sym_get_rev_dep(sym) != no)
+               str_printf(r, "Enforced value: %s (see Selected by:)\n",
+                             sym_get_rev_dep(sym) == mod ? "[m] or [y]" : "[y]");
+       if (sym_get_visibility(sym) == no)
+               str_append(r, _("None of the prompts active, default value assigned\n"));
        for_all_prompts(sym, prop)
                get_prompt_str(r, prop);

> > The function names are maybe suboptimal, I agree.
> 
> The variable name is already correct, it's the visibility value of a 
> symbol not its maximum. In the case of the "if EMBEDDED" then individual 
> menu entries can still be visible, if any child entry is visible (see 
> menu_is_visible()). 

Changed function names to sym_get_rev_dep() and sym_get_visibility().
Shouldn't I move them from symbol.c and lkc_proto.h into lkc.h? They would
fit into the section with static inline one-liners.

Bye,
     Matej.

[PATCH] kconfig: menuconfig: change "---" items to "-*-", "-M-" or "- -"

[Matej Laitl]

This little patch changes notation of unchangeable menuconfig items to always
reflect the current state of inclusion of a feature.

Signed-off-by: Matěj Laitl <strohel@gmail.com>
---
Hi again, Sam, Roman, Randy.

Instead of incorporating all changes into a single patch, I decided to split 
things into more rather independent patches.

Expect another one extending show_help(). (which ATM needs more polishing 
before being posted)

 scripts/kconfig/mconf.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
index bc5854e..38ffbe1 100644
--- a/scripts/kconfig/mconf.c
+++ b/scripts/kconfig/mconf.c
@@ -542,14 +542,19 @@ static void build_conf(struct menu *menu)
 			return;
 		}
 	} else {
+		val = sym_get_tristate_value(sym);
+		switch (val) {
+			case yes: ch = '*'; break;
+			case mod: ch = 'M'; break;
+			default:  ch = ' '; break;
+		}
 		if (menu == current_menu) {
-			item_make("---%*c%s", indent + 1, ' ', menu_get_prompt(menu));
+			item_make("-%c-%*c%s", ch, indent + 1, ' ', menu_get_prompt(menu));
 			item_set_tag(':');
 			item_set_data(menu);
 			goto conf_childs;
 		}
 		child_count++;
-		val = sym_get_tristate_value(sym);
 		if (sym_is_choice_value(sym) && val == yes) {
 			item_make("   ");
 			item_set_tag(':');
@@ -558,22 +563,17 @@ static void build_conf(struct menu *menu)
 			switch (type) {
 			case S_BOOLEAN:
 				if (sym_is_changable(sym))
-					item_make("[%c]", val == no ? ' ' : '*');
+					item_make("[%c]", ch);
 				else
-					item_make("---");
+					item_make("-%c-", ch);
 				item_set_tag('t');
 				item_set_data(menu);
 				break;
 			case S_TRISTATE:
-				switch (val) {
-				case yes: ch = '*'; break;
-				case mod: ch = 'M'; break;
-				default:  ch = ' '; break;
-				}
 				if (sym_is_changable(sym))
 					item_make("<%c>", ch);
 				else
-					item_make("---");
+					item_make("-%c-", ch);
 				item_set_tag('t');
 				item_set_data(menu);
 				break;
-- 
1.5.1.6