[PATCH] fix undesirable side effect of adding "visible" menu attribute

[PATCH] fix undesirable side effect of adding "visible" menu attribute

[Jan Beulich]

This lead to non-selected, non-user-selectable options to be written
out to .config. This is not only pointless, but also preventing the
user to be prompted should any of those options eventually become
visible (e.g. by de-selecting the *_AUTO options the "visible"
attribute was added for.

Furthermore it is quite logical for the "visible" attribute of a menu
to control the visibility of all contained prompts, which is what the
patch does.

Signed-off-by: Jan Beulich <jbeulich@novell.com>

---
 scripts/kconfig/menu.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

--- 2.6.37-rc5/scripts/kconfig/menu.c
+++ 2.6.37-rc5-kconfig-propagate-visibility/scripts/kconfig/menu.c
@@ -140,6 +140,20 @@ struct property *menu_add_prop(enum prop
 		}
 		if (current_entry->prompt && current_entry != &rootmenu)
 			prop_warn(prop, "prompt redefined");
+
+		/* Apply all upper menus' visibilities to actual prompts. */
+		if(type == P_PROMPT) {
+			struct menu *menu = current_entry;
+
+			while ((menu = menu->parent) != NULL) {
+				if (!menu->visibility)
+					continue;
+				prop->visible.expr
+					= expr_alloc_and(prop->visible.expr,
+							 menu->visibility);
+			}
+		}
+
 		current_entry->prompt = prop;
 	}
 	prop->text = prompt;

Re: [PATCH] fix undesirable side effect of adding "visible" menu attribute

[Arnaud Lacombe]

Hi,

On Thu, Dec 9, 2010 at 3:11 AM, Jan Beulich <JBeulich@novell.com> wrote:
> This lead to non-selected, non-user-selectable options to be written
> out to .config. This is not only pointless, but also preventing the
> user to be prompted should any of those options eventually become
> visible (e.g. by de-selecting the *_AUTO options the "visible"
> attribute was added for.
>
> Furthermore it is quite logical for the "visible" attribute of a menu
> to control the visibility of all contained prompts, which is what the
> patch does.
>
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>
> ---
>  scripts/kconfig/menu.c |   14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> --- 2.6.37-rc5/scripts/kconfig/menu.c
> +++ 2.6.37-rc5-kconfig-propagate-visibility/scripts/kconfig/menu.c
> @@ -140,6 +140,20 @@ struct property *menu_add_prop(enum prop
>                }
>                if (current_entry->prompt && current_entry != &rootmenu)
>                        prop_warn(prop, "prompt redefined");
> +
> +               /* Apply all upper menus' visibilities to actual prompts. */
> +               if(type == P_PROMPT) {
> +                       struct menu *menu = current_entry;
> +
> +                       while ((menu = menu->parent) != NULL) {
> +                               if (!menu->visibility)
> +                                       continue;
> +                               prop->visible.expr
> +                                       = expr_alloc_and(prop->visible.expr,
> +                                                        menu->visibility);
> +                       }
> +               }
> +
Shouldn't this better to be done in menu_finalize() ?

I'm not either a big fan of testing `menu->visibility' truth, it is an
expression which should be expended by the proper expr_* accessor to
have less things to fix later.

 - Arnaud

>                current_entry->prompt = prop;
>        }
>        prop->text = prompt;
>
>
>
>

Re: [PATCH] fix undesirable side effect of adding "visible" menu attribute

[Arnaud Lacombe]

Hi,

On Thu, Dec 9, 2010 at 10:58 AM, Arnaud Lacombe <lacombar@gmail.com> wrote:
[..]
> I'm not either a big fan of testing `menu->visibility' truth, it is an
> expression which should be expended by the proper expr_* accessor to
> have less things to fix later.
>
gniarf, forget about that paragraph, I'm not fully awaken :/

What I thought about was the case where the visible property would
explicitly default to 'y' in which case the expression will have to be
simplified/manipulated, in that case, menu_finalize() will be the
place to do that, so this come back to my first comment.

 - Arnaud

Re: [PATCH] fix undesirable side effect of adding "visible" menu attribute

[Jan Beulich]

>>> On 09.12.10 at 17:07, Arnaud Lacombe <lacombar@gmail.com> wrote:
> On Thu, Dec 9, 2010 at 10:58 AM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> [..]
>> I'm not either a big fan of testing `menu->visibility' truth, it is an
>> expression which should be expended by the proper expr_* accessor to
>> have less things to fix later.
>>
> gniarf, forget about that paragraph, I'm not fully awaken :/
> 
> What I thought about was the case where the visible property would
> explicitly default to 'y' in which case the expression will have to be
> simplified/manipulated, in that case, menu_finalize() will be the
> place to do that, so this come back to my first comment.

I'm not sure in menu_finalize() you can properly/easily deal with
multiple prompts associated with the same symbol under different
menus.

Anyway, the patch I proposed works (for me), and I'm hoping
to get it in unless someone (you?) comes up with a better one.
Not fixing the (presumably unintended) side effect of your
earlier patch seems undesirable to me.

Jan

Re: [PATCH] fix undesirable side effect of adding "visible" menu attribute

[Jan Beulich]

>>> On 09.12.10 at 16:58, Arnaud Lacombe <lacombar@gmail.com> wrote:
> On Thu, Dec 9, 2010 at 3:11 AM, Jan Beulich <JBeulich@novell.com> wrote:
>> --- 2.6.37-rc5/scripts/kconfig/menu.c
>> +++ 2.6.37-rc5-kconfig-propagate-visibility/scripts/kconfig/menu.c
>> @@ -140,6 +140,20 @@ struct property *menu_add_prop(enum prop
>>                }
>>                if (current_entry->prompt && current_entry != &rootmenu)
>>                        prop_warn(prop, "prompt redefined");
>> +
>> +               /* Apply all upper menus' visibilities to actual prompts. */
>> +               if(type == P_PROMPT) {
>> +                       struct menu *menu = current_entry;
>> +
>> +                       while ((menu = menu->parent) != NULL) {
>> +                               if (!menu->visibility)
>> +                                       continue;
>> +                               prop->visible.expr
>> +                                       = expr_alloc_and(prop->visible.expr,
>> +                                                        menu->visibility);
>> +                       }
>> +               }
>> +
> Shouldn't this better to be done in menu_finalize() ?

I don't know, I just looked for a place where I could get it done
in a reasonably simple way.

> I'm not either a big fan of testing `menu->visibility' truth, it is an
> expression which should be expended by the proper expr_* accessor to
> have less things to fix later.

Not sure what you're telling me here? What accessor?

Jan

Re: [PATCH] fix undesirable side effect of adding "visible" menu attribute

[Michal Marek]

On Thu, Dec 09, 2010 at 08:11:38AM +0000, Jan Beulich wrote:
> This lead to non-selected, non-user-selectable options to be written
> out to .config. This is not only pointless, but also preventing the
> user to be prompted should any of those options eventually become
> visible (e.g. by de-selecting the *_AUTO options the "visible"
> attribute was added for.
> 
> Furthermore it is quite logical for the "visible" attribute of a menu
> to control the visibility of all contained prompts, which is what the
> patch does.
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>

It also fixes the issue that all{mod,yes}config set some options from
the invisible menus that cannot be selected interactively (e.g.
I2C_ALGOPCF, that is only selected by I2C_ELEKTOR on !SMP), so this
patch is

Acked-by: Michal Marek <mmarek@suse.cz>

unless Arnaud has a good reason not to take it.

Michal

Re: [PATCH] fix undesirable side effect of adding "visible" menu attribute

[Arnaud Lacombe]

Hi,

On Thu, Dec 16, 2010 at 4:26 PM, Michal Marek <mmarek@suse.cz> wrote:
> On Thu, Dec 09, 2010 at 08:11:38AM +0000, Jan Beulich wrote:
>> This lead to non-selected, non-user-selectable options to be written
>> out to .config. This is not only pointless, but also preventing the
>> user to be prompted should any of those options eventually become
>> visible (e.g. by de-selecting the *_AUTO options the "visible"
>> attribute was added for.
>>
>> Furthermore it is quite logical for the "visible" attribute of a menu
>> to control the visibility of all contained prompts, which is what the
>> patch does.
>>
>> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>
> It also fixes the issue that all{mod,yes}config set some options from
> the invisible menus that cannot be selected interactively (e.g.
> I2C_ALGOPCF, that is only selected by I2C_ELEKTOR on !SMP), so this
> patch is
>
> Acked-by: Michal Marek <mmarek@suse.cz>
>
> unless Arnaud has a good reason not to take it.
>
Actually, I wanted to keep the parsing and tree construction as simple
as possible. Fix-up like that should really happen in menu_finalize()
as we will anyway traverse all the menus once again there and it is
already doing all kind of dependency simplification, so that would
keep everything at the same place.

Say that if I do not show up with a version which would move this in
menu_finalize() by the end of the week (sooner if you want that for
.37), feel free to forget about this mail. Hopefully, I'll not move
back to Linux this week-end, so I should have the time to do that.

 - Arnaud

Re: [PATCH] fix undesirable side effect of adding "visible" menu attribute

[Arnaud Lacombe]

Hi all,

On Thu, Dec 16, 2010 at 6:49 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> Hi,
>
> On Thu, Dec 16, 2010 at 4:26 PM, Michal Marek <mmarek@suse.cz> wrote:
>> On Thu, Dec 09, 2010 at 08:11:38AM +0000, Jan Beulich wrote:
>>> This lead to non-selected, non-user-selectable options to be written
>>> out to .config. This is not only pointless, but also preventing the
>>> user to be prompted should any of those options eventually become
>>> visible (e.g. by de-selecting the *_AUTO options the "visible"
>>> attribute was added for.
>>>
>>> Furthermore it is quite logical for the "visible" attribute of a menu
>>> to control the visibility of all contained prompts, which is what the
>>> patch does.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>>
>> It also fixes the issue that all{mod,yes}config set some options from
>> the invisible menus that cannot be selected interactively (e.g.
>> I2C_ALGOPCF, that is only selected by I2C_ELEKTOR on !SMP), so this
>> patch is
>>
>> Acked-by: Michal Marek <mmarek@suse.cz>
>>
>> unless Arnaud has a good reason not to take it.
>>
> Actually, I wanted to keep the parsing and tree construction as simple
> as possible. Fix-up like that should really happen in menu_finalize()
> as we will anyway traverse all the menus once again there and it is
> already doing all kind of dependency simplification, so that would
> keep everything at the same place.
>
hum, I just thought of something. Instead of adding more complexity to
the menu children, why would {,m,n,q}conf descend to these submenus at
first. they should just ignore all the childs if the parent is not
meant to be visible and go to the next node on the same level. When I
close the front door of my house, I do expect people to stop right
there, not continue to check any door I may have left open inside.

 - Arnaud

Re: [PATCH] fix undesirable side effect of adding "visible" menu attribute

[Jan Beulich]

>>> On 17.12.10 at 03:03, Arnaud Lacombe <lacombar@gmail.com> wrote:
> hum, I just thought of something. Instead of adding more complexity to
> the menu children, why would {,m,n,q}conf descend to these submenus at
> first. they should just ignore all the childs if the parent is not
> meant to be visible and go to the next node on the same level. When I
> close the front door of my house, I do expect people to stop right
> there, not continue to check any door I may have left open inside.

Isn't that what's happening? I don't think the problem is with the
frontends - it's the backend that writes out settings of options it
didn't before (and shouldn't).

Jan

Re: [PATCH] fix undesirable side effect of adding "visible" menu attribute

[Michal Marek]

On 17.12.2010 00:49, Arnaud Lacombe wrote:
> Hi,
> 
> On Thu, Dec 16, 2010 at 4:26 PM, Michal Marek <mmarek@suse.cz> wrote:
>> On Thu, Dec 09, 2010 at 08:11:38AM +0000, Jan Beulich wrote:
>>> This lead to non-selected, non-user-selectable options to be written
>>> out to .config. This is not only pointless, but also preventing the
>>> user to be prompted should any of those options eventually become
>>> visible (e.g. by de-selecting the *_AUTO options the "visible"
>>> attribute was added for.
>>>
>>> Furthermore it is quite logical for the "visible" attribute of a menu
>>> to control the visibility of all contained prompts, which is what the
>>> patch does.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@novell.com>
>>
>> It also fixes the issue that all{mod,yes}config set some options from
>> the invisible menus that cannot be selected interactively (e.g.
>> I2C_ALGOPCF, that is only selected by I2C_ELEKTOR on !SMP), so this
>> patch is
>>
>> Acked-by: Michal Marek <mmarek@suse.cz>
>>
>> unless Arnaud has a good reason not to take it.
>>
> Actually, I wanted to keep the parsing and tree construction as simple
> as possible. Fix-up like that should really happen in menu_finalize()
> as we will anyway traverse all the menus once again there and it is
> already doing all kind of dependency simplification, so that would
> keep everything at the same place.
> 
> Say that if I do not show up with a version which would move this in
> menu_finalize() by the end of the week (sooner if you want that for
> .37), feel free to forget about this mail. Hopefully, I'll not move
> back to Linux this week-end, so I should have the time to do that.

I'd like to have _some_ fix in 2.6.37, if Linus takes it. So unless you
come up with an alternate patch RSN, I'll apply Jan's patch and send a
pull request to Linus.

Michal

Re: [PATCH] fix undesirable side effect of adding "visible" menu attribute

[Arnaud Lacombe]

Hi,

On Wed, Dec 29, 2010 at 4:19 AM, Michal Marek <mmarek@suse.cz> wrote:
> I'd like to have _some_ fix in 2.6.37, if Linus takes it. So unless you
> come up with an alternate patch RSN, I'll apply Jan's patch and send a
> pull request to Linus.
>
Please go on, the other solutions I had is more intrusive, and still
need to be completed.

 - Arnaud

Re: [PATCH] fix undesirable side effect of adding "visible" menu attribute

[Michal Marek]

On 29.12.2010 16:17, Arnaud Lacombe wrote:
> Hi,
> 
> On Wed, Dec 29, 2010 at 4:19 AM, Michal Marek <mmarek@suse.cz> wrote:
>> I'd like to have _some_ fix in 2.6.37, if Linus takes it. So unless you
>> come up with an alternate patch RSN, I'll apply Jan's patch and send a
>> pull request to Linus.
>>
> Please go on, the other solutions I had is more intrusive, and still
> need to be completed.

OK, applied Jan's patch to kbuild-2.6.git#rc-fixes.

Michal