On Thu, Feb 29, 2024 at 10:57 AM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote: > > On Thu, Feb 29, 2024 at 1:36 AM Matthew Bystrin <dev.mbstr@xxxxxxxxx> wrote: > > > > On Wed Feb 28, 2024 at 9:00 AM MSK, Tomasz Figa wrote: > > > When hidden options are toggled on (using 'z'), the number of options > > > on the screen can be overwhelming and may make it hard to distinguish > > > between available and hidden ones. Make them easier to distinguish by > > > displaying the hidden one with a different color (COLOR_YELLOW for color > > > themes and A_DIM for mono). > > > > > > Signed-off-by: Tomasz Figa <tfiga@xxxxxxxxxxxx> > > > --- > > > scripts/kconfig/lxdialog/dialog.h | 5 +++++ > > > scripts/kconfig/lxdialog/menubox.c | 12 ++++++++---- > > > scripts/kconfig/lxdialog/util.c | 19 +++++++++++++++++++ > > > scripts/kconfig/mconf.c | 18 ++++++++++++++++++ > > > 4 files changed, 50 insertions(+), 4 deletions(-) > > > > > > Changes from v1: > > > (https://patchwork.kernel.org/project/linux-kbuild/patch/20231228054630.3595093-1-tfiga@xxxxxxxxxxxx/) > > > * Replaced A_DIM for color themes with COLOR_YELLOW, because the former > > > has no effect to black text on some commonly used terminals, e.g. > > > gnome-terminal, foot. Reported by Masahiro Yamada and Nicolas Schier. > > > I ended up with COLOR_YELLOW, as it seems to look comparatively dim > > > with mutliple light and dark color themes in Chromium hterm and > > > gnome-terminal. > > > > Thanks! Run a quick tests in xterm. Looks neat! > > > > Is there a reason to set hidden flag in all of the _if_ and _switch_ statements > > in the build_conf() function? Could similar be done in a more generic way? For > > example: > > > > visible = menu_is_visible(menu); > > if (!visible) > > item_set_hidden(TRUE); > > > > Or this approach will bring some negative side effects ? > > > > > I guess he just inserted item_set_hidden() where he saw item_make(). > > > Since build_conf() resources to itself, the code flow > is difficult to track. > > > You can safely factor it out in some places (for example, just blow), > but that does not make a big difference. > > > > diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c > index b7e08ec98717..ba0f177121ed 100644 > --- a/scripts/kconfig/mconf.c > +++ b/scripts/kconfig/mconf.c > @@ -546,16 +546,15 @@ static void build_conf(struct menu *menu) > } > item_set_tag('t'); > item_set_data(menu); > - if (!visible) > - item_set_hidden(TRUE); > } else { > item_make(" "); > item_set_tag(def_menu ? 't' : ':'); > item_set_data(menu); > - if (!visible) > - item_set_hidden(TRUE); I wanted to be consistent with the current code, which already has the same item_set_data(menu) in both branches. I'm okay with either. Do you want me to resend with this change? Best regards, Tomasz > } > > + if (!visible) > + item_set_hidden(TRUE); > + > item_add_str("%*c%s", indent + 1, ' ', menu_get_prompt(menu)); > if (val == yes) { > if (def_menu) { > > > > > > > > > -- > Best Regards > Masahiro Yamada
