[PATCH] kconfig qconf: Add grey background for hidden options

[PATCH] kconfig qconf: Add grey background for hidden options

[Maxime Chretien]

This is useful to see which configuration parameters can be edited
or not when "Show All Options" is enabled.

Signed-off-by: Maxime Chretien <maxime.chretien@bootlin.com>
---
 scripts/kconfig/qconf.cc | 7 +++++++
 scripts/kconfig/qconf.h  | 4 ++++
 2 files changed, 11 insertions(+)

diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
index c0ac8f7b5f1a..be9ff4651da1 100644
--- a/scripts/kconfig/qconf.cc
+++ b/scripts/kconfig/qconf.cc
@@ -208,6 +208,13 @@ void ConfigItem::updateMenu(void)
 	}
 	if (!sym_has_value(sym) && visible)
 		prompt += " (NEW)";
+
+	if(!visible) {
+		setBackground(promptColIdx, QBrush(QColor("#E0E0E0")));
+	} else {
+		setBackground(promptColIdx, QBrush());
+	}
+
 set_prompt:
 	setText(promptColIdx, prompt);
 }
diff --git a/scripts/kconfig/qconf.h b/scripts/kconfig/qconf.h
index c879d79ce817..79e47e8c1ae7 100644
--- a/scripts/kconfig/qconf.h
+++ b/scripts/kconfig/qconf.h
@@ -174,6 +174,10 @@ class ConfigItem : public QTreeWidgetItem {
 	{
 		return Parent::text(idx);
 	}
+	void setBackground(colIdx idx, const QBrush& brush)
+	{
+		Parent::setBackground(idx, brush);
+	}
 	void setPixmap(colIdx idx, const QIcon &icon)
 	{
 		Parent::setIcon(idx, icon);
-- 
2.27.0

Re: [PATCH] kconfig qconf: Add grey background for hidden options

[Masahiro Yamada]

On Wed, Jul 8, 2020 at 10:30 PM Maxime Chretien
<maxime.chretien@bootlin.com> wrote:
>
> This is useful to see which configuration parameters can be edited
> or not when "Show All Options" is enabled.
>
> Signed-off-by: Maxime Chretien <maxime.chretien@bootlin.com>
> ---
>  scripts/kconfig/qconf.cc | 7 +++++++
>  scripts/kconfig/qconf.h  | 4 ++++
>  2 files changed, 11 insertions(+)


I like the idea, but
maybe this patch could be improved?

For example, in the following test code,
BAR is correctly painted grey when CONFIG_FOO=n,
but "my menu" is always white despite of
"depends on FOO"

----(test code)------

config FOO
       bool "foo"

config BAR
       bool "bar"
       depends on FOO

menu "my menu"
       depends on FOO

endmenu

-------(test code end)----







> diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
> index c0ac8f7b5f1a..be9ff4651da1 100644
> --- a/scripts/kconfig/qconf.cc
> +++ b/scripts/kconfig/qconf.cc
> @@ -208,6 +208,13 @@ void ConfigItem::updateMenu(void)
>         }
>         if (!sym_has_value(sym) && visible)
>                 prompt += " (NEW)";
> +
> +       if(!visible) {
> +               setBackground(promptColIdx, QBrush(QColor("#E0E0E0")));
> +       } else {
> +               setBackground(promptColIdx, QBrush());
> +       }
> +


I think all the columns should be grey-grounded.
Please note you can click other columns to
toggle y/m/n.


How about something like this?



QBrush brush;

if (visible)
        brush = QBrush()
else
        brush = QBrush(QColor("#E0E0E0"));

setBackground(promptColIdx, brush);
setBackground(nameColIdx, brush);
setBackground(noColIdx, brush);
setBackground(modColIdx, brush);
setBackground(yesColIdx, brush);
setBackground(dataColIdx, brush);




>  set_prompt:
>         setText(promptColIdx, prompt);
>  }
> diff --git a/scripts/kconfig/qconf.h b/scripts/kconfig/qconf.h
> index c879d79ce817..79e47e8c1ae7 100644
> --- a/scripts/kconfig/qconf.h
> +++ b/scripts/kconfig/qconf.h
> @@ -174,6 +174,10 @@ class ConfigItem : public QTreeWidgetItem {
>         {
>                 return Parent::text(idx);
>         }
> +       void setBackground(colIdx idx, const QBrush& brush)
> +       {
> +               Parent::setBackground(idx, brush);
> +       }

I do not understand why this wrapper is useful...


>         void setPixmap(colIdx idx, const QIcon &icon)
>         {
>                 Parent::setIcon(idx, icon);
> --
> 2.27.0
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH] kconfig qconf: Add grey background for hidden options

[Maxime Chretien]

On 07/08/2020 15:54, Masahiro Yamada wrote:
> On Wed, Jul 8, 2020 at 10:30 PM Maxime Chretien
> <maxime.chretien@bootlin.com>  wrote:
>> This is useful to see which configuration parameters can be edited
>> or not when "Show All Options" is enabled.
>>
>> Signed-off-by: Maxime Chretien<maxime.chretien@bootlin.com>
>> ---
>>   scripts/kconfig/qconf.cc | 7 +++++++
>>   scripts/kconfig/qconf.h  | 4 ++++
>>   2 files changed, 11 insertions(+)
> I like the idea, but
> maybe this patch could be improved?
>
> For example, in the following test code,
> BAR is correctly painted grey when CONFIG_FOO=n,
> but "my menu" is always white despite of
> "depends on FOO"
>
> ----(test code)------
>
> config FOO
>         bool "foo"
>
> config BAR
>         bool "bar"
>         depends on FOO
>
> menu "my menu"
>         depends on FOO
>
> endmenu
>
> -------(test code end)----


It could probably be improved.
Maybe the "visible" state of the menu can be changed
or maybe we should use another element in the test
to grey out menus that needs to be greyed out.


>> diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
>> index c0ac8f7b5f1a..be9ff4651da1 100644
>> --- a/scripts/kconfig/qconf.cc
>> +++ b/scripts/kconfig/qconf.cc
>> @@ -208,6 +208,13 @@ void ConfigItem::updateMenu(void)
>>          }
>>          if (!sym_has_value(sym) && visible)
>>                  prompt += " (NEW)";
>> +
>> +       if(!visible) {
>> +               setBackground(promptColIdx, QBrush(QColor("#E0E0E0")));
>> +       } else {
>> +               setBackground(promptColIdx, QBrush());
>> +       }
>> +
> I think all the columns should be grey-grounded.
> Please note you can click other columns to
> toggle y/m/n.
>
>
> How about something like this?
>
>
>
> QBrush brush;
>
> if (visible)
>          brush = QBrush()
> else
>          brush = QBrush(QColor("#E0E0E0"));
>
> setBackground(promptColIdx, brush);
> setBackground(nameColIdx, brush);
> setBackground(noColIdx, brush);
> setBackground(modColIdx, brush);
> setBackground(yesColIdx, brush);
> setBackground(dataColIdx, brush);


Yes I agree should be better like that so that we can
see what's grey or not even if we have scrolled far
from the first column.


>
>
>>   set_prompt:
>>          setText(promptColIdx, prompt);
>>   }
>> diff --git a/scripts/kconfig/qconf.h b/scripts/kconfig/qconf.h
>> index c879d79ce817..79e47e8c1ae7 100644
>> --- a/scripts/kconfig/qconf.h
>> +++ b/scripts/kconfig/qconf.h
>> @@ -174,6 +174,10 @@ class ConfigItem : public QTreeWidgetItem {
>>          {
>>                  return Parent::text(idx);
>>          }
>> +       void setBackground(colIdx idx, const QBrush& brush)
>> +       {
>> +               Parent::setBackground(idx, brush);
>> +       }
> I do not understand why this wrapper is useful...
>


Well we need to use "Parent" to have access to the "setBackground" function
and I thought it was the way to use it has it's done with "setPixmap" 
just under it.

Maybe there is a better way to do that, I'm a beginner in linux development
so I'm not totally aware of all the rules and ways to add new things.


>>          void setPixmap(colIdx idx, const QIcon &icon)
>>          {
>>                  Parent::setIcon(idx, icon);
>> --
>> 2.27.0
>>


Thanks for reviewing this.

Best regards
Maxime Chretien