2010-07-27 14:46:48

by trapDoor

[permalink] [raw]
Subject: Small typo in kernel [current source from git] .config option

Hello,
I've just got lucky and found this little typo under "Power management
and ACPI options" ---> "ACPI (Advanced Configuration and Power
Interface) Support":
hArdware Error Device

Here is my fix:
Hardware Error Device
:)

BTW, first time I've used nconfig instead of menuconfig. And I like it.

--
Regards
trapDoor


2010-07-27 15:50:18

by Justin P. Mattock

[permalink] [raw]
Subject: Re: Small typo in kernel [current source from git] .config option

On 07/27/2010 07:46 AM, trapDoor wrote:
> Hello,
> I've just got lucky and found this little typo under "Power management
> and ACPI options" ---> "ACPI (Advanced Configuration and Power
> Interface) Support":
> hArdware Error Device
>
> Here is my fix:
> Hardware Error Device
> :)
>
> BTW, first time I've used nconfig instead of menuconfig. And I like it.
>

probably should create a patch, and send it to [email protected]

Justin P. Mattock

2010-07-27 16:35:20

by trapDoor

[permalink] [raw]
Subject: Re: Small typo in kernel [current source from git] .config option

On Tue, Jul 27, 2010 at 4:50 PM, Justin P. Mattock
<[email protected]> wrote:
> On 07/27/2010 07:46 AM, trapDoor wrote:
>>
>> Hello,
>> I've just got lucky and found this little typo under "Power management
>> and ACPI options" ?---> ?"ACPI (Advanced Configuration and Power
>> Interface) Support":
>> hArdware Error Device
>>
>> Here is my fix:
>> Hardware Error Device
>> :)
>>
>> BTW, first time I've used nconfig instead of menuconfig. And I like it.
>>
>
> probably should create a patch, and send it to [email protected]
>
> Justin P. Mattock
>

Sure, but I don't know which script generates that part of config and
... how to do the patch :| (Sorry, I'm not even a greenhorn on the
programming field).
Just cc-eding [email protected] for now ..

--
Regards
trapDoor

2010-07-27 16:45:49

by Justin P. Mattock

[permalink] [raw]
Subject: Re: Small typo in kernel [current source from git] .config option

On 07/27/2010 09:35 AM, trapDoor wrote:
> On Tue, Jul 27, 2010 at 4:50 PM, Justin P. Mattock
> <[email protected]> wrote:
>> On 07/27/2010 07:46 AM, trapDoor wrote:
>>>
>>> Hello,
>>> I've just got lucky and found this little typo under "Power management
>>> and ACPI options" ---> "ACPI (Advanced Configuration and Power
>>> Interface) Support":
>>> hArdware Error Device
>>>
>>> Here is my fix:
>>> Hardware Error Device
>>> :)
>>>
>>> BTW, first time I've used nconfig instead of menuconfig. And I like it.
>>>
>>
>> probably should create a patch, and send it to [email protected]
>>
>> Justin P. Mattock
>>
>
> Sure, but I don't know which script generates that part of config and
> ... how to do the patch :| (Sorry, I'm not even a greenhorn on the
> programming field).
> Just cc-eding [email protected] for now ..
>

That's alright.. I'm still a newbie over here...

read Documentation/SubmittingPatches

there the instructions on how to create a perfect patch is there.
then when done make sure you use scripts/checkpatch to check you patch
then when all good use scripts/getmaintainer.pl to find the appropriate
people to send it too.

and voila you'll be sending patches to the kernel like there's no
tomorrow...


Justin P. Mattock

2010-07-27 16:55:51

by Sam Ravnborg

[permalink] [raw]
Subject: Re: Small typo in kernel [current source from git] .config option

On Tue, Jul 27, 2010 at 03:46:45PM +0100, trapDoor wrote:
> Hello,
> I've just got lucky and found this little typo under "Power management
> and ACPI options" ---> "ACPI (Advanced Configuration and Power
> Interface) Support":
> hArdware Error Device
>
> Here is my fix:
> Hardware Error Device
> :)
>
> BTW, first time I've used nconfig instead of menuconfig. And I like it.

The capital "A" is nconfigs way to tell you that you can use
"a" as shortcut to reach the menu.

Sam

2010-07-27 17:12:11

by trapDoor

[permalink] [raw]
Subject: Re: Small typo in kernel [current source from git] .config option

On Tue, Jul 27, 2010 at 5:55 PM, Sam Ravnborg <[email protected]> wrote:
> On Tue, Jul 27, 2010 at 03:46:45PM +0100, trapDoor wrote:
>> Hello,
>> I've just got lucky and found this little typo under "Power management
>> and ACPI options" ?---> "ACPI (Advanced Configuration and Power
>> Interface) Support":
>> hArdware Error Device
>>
>> Here is my fix:
>> Hardware Error Device
>> :)
>>
>> BTW, first time I've used nconfig instead of menuconfig. And I like it.
>
> The capital "A" is nconfigs way to tell you that you can use
> "a" as shortcut to reach the menu.
>
> ? ? ? ?Sam
>

Rignt! So it's just a 'mnemonic' and only appears like that in nconfig
(I see that now). I'd guess that if I've ever made any use of
mnemonics here.

Thanks for clarifying and sorry for making needless 'noise'.

--
Regards
trapDoor

2010-07-28 11:03:09

by Michal Marek

[permalink] [raw]
Subject: Re: Small typo in kernel [current source from git] .config option

On 27.7.2010 19:12, trapDoor wrote:
> On Tue, Jul 27, 2010 at 5:55 PM, Sam Ravnborg <[email protected]> wrote:
>> On Tue, Jul 27, 2010 at 03:46:45PM +0100, trapDoor wrote:
>>> Hello,
>>> I've just got lucky and found this little typo under "Power management
>>> and ACPI options" ---> "ACPI (Advanced Configuration and Power
>>> Interface) Support":
>>> hArdware Error Device
>>>
>>> Here is my fix:
>>> Hardware Error Device
>>> :)
>>>
>>> BTW, first time I've used nconfig instead of menuconfig. And I like it.
>>
>> The capital "A" is nconfigs way to tell you that you can use
>> "a" as shortcut to reach the menu.
>
> Rignt! So it's just a 'mnemonic' and only appears like that in nconfig
> (I see that now). I'd guess that if I've ever made any use of
> mnemonics here.

For completeness - This affects any option that starts with H, Y, M or
N, as these letters are taken (they stand for Help, Yes, Module and No,
respectively). Unfortunatelly, there doesn't seem to be a way to
highlight single letters in an ncurses menu, hence the strange
cApitalization :(.

Michal

2010-07-28 12:17:45

by trapDoor

[permalink] [raw]
Subject: Re: Small typo in kernel [current source from git] .config option

On Wed, Jul 28, 2010 at 12:02 PM, Michal Marek <[email protected]> wrote:
> On 27.7.2010 19:12, trapDoor wrote:
>> On Tue, Jul 27, 2010 at 5:55 PM, Sam Ravnborg <[email protected]> wrote:
>>> On Tue, Jul 27, 2010 at 03:46:45PM +0100, trapDoor wrote:
>>>> Hello,
>>>> I've just got lucky and found this little typo under "Power management
>>>> and ACPI options" ?---> "ACPI (Advanced Configuration and Power
>>>> Interface) Support":
>>>> hArdware Error Device
>>>>
>>>> Here is my fix:
>>>> Hardware Error Device
>>>> :)
>>>>
>>>> BTW, first time I've used nconfig instead of menuconfig. And I like it.
>>>
>>> The capital "A" is nconfigs way to tell you that you can use
>>> "a" as shortcut to reach the menu.
>>
>> Rignt! So it's just a 'mnemonic' and only appears like that in nconfig
>> (I see that now). I'd guess that if I've ever made any use of
>> mnemonics here.
>
> For completeness - This affects any option that starts with H, Y, M or
> N, as these letters are taken (they stand for Help, Yes, Module and No,
> respectively). Unfortunatelly, there doesn't seem to be a way to
> highlight single letters in an ncurses menu, hence the strange
> cApitalization :(.
>
> Michal
>

I wonder how is this resolved in other console applications such as
Midnight Commander. Does it have its own implementation for
highlighting characters (it uses coloured mnemonics e.g. in menus) or
it's based on some common Linux/Unix library which perhaps could be
used for the nconfig as well? Unless MC depends on any part of X env.
but I don't think it does. On the project's official website it says:

"Midnight Commander is based on versatile text interfaces, such as
Ncurses or S-Lang, which allows it to work on a regular console,
inside an X Window terminal, over SSH connections and all kinds of
remote shells."

So either ncurses or s-lang, no mention about other dependences in
their documentation. Also by looking at some forums, when people have
problems with installing MC it's usually due to missing s-lang

--
Regards
trapDoor

2010-07-28 15:41:53

by Michal Marek

[permalink] [raw]
Subject: Re: Small typo in kernel [current source from git] .config option

(adding Nir, who wrote nconfig, to CC).

On 28.7.2010 14:17, trapDoor wrote:
> On Wed, Jul 28, 2010 at 12:02 PM, Michal Marek <[email protected]> wrote:
>> respectively). Unfortunatelly, there doesn't seem to be a way to
>> highlight single letters in an ncurses menu, hence the strange
>> cApitalization :(.
>
> I wonder how is this resolved in other console applications such as
> Midnight Commander. Does it have its own implementation for
> highlighting characters (it uses coloured mnemonics e.g. in menus) or
> it's based on some common Linux/Unix library which perhaps could be
> used for the nconfig as well?

The problem is that nconfig uses the higher-level ncurses libraries
libmenu and libpanel, which make some things easier, but aren't as
flexible as using ncurses directly. Patches are welcome of course :-).

Michal

2010-07-28 18:55:12

by Sam Ravnborg

[permalink] [raw]
Subject: Re: Small typo in kernel [current source from git] .config option

On Wed, Jul 28, 2010 at 05:41:47PM +0200, Michal Marek wrote:
> (adding Nir, who wrote nconfig, to CC).
>
> On 28.7.2010 14:17, trapDoor wrote:
> > On Wed, Jul 28, 2010 at 12:02 PM, Michal Marek <[email protected]> wrote:
> >> respectively). Unfortunatelly, there doesn't seem to be a way to
> >> highlight single letters in an ncurses menu, hence the strange
> >> cApitalization :(.
> >
> > I wonder how is this resolved in other console applications such as
> > Midnight Commander. Does it have its own implementation for
> > highlighting characters (it uses coloured mnemonics e.g. in menus) or
> > it's based on some common Linux/Unix library which perhaps could be
> > used for the nconfig as well?
>
> The problem is that nconfig uses the higher-level ncurses libraries
> libmenu and libpanel, which make some things easier, but aren't as
> flexible as using ncurses directly. Patches are welcome of course :-).

The capitilazing is the main reason why I have
not recommended nconfig to be default.

And I doubt the shortcut letters are used that often.
One idea could be to drop the shortcut for the individual menus.

Sam

2010-07-29 05:04:55

by Nir Tzachar

[permalink] [raw]
Subject: Re: Small typo in kernel [current source from git] .config option

Hello.

On Wed, Jul 28, 2010 at 9:55 PM, Sam Ravnborg <[email protected]> wrote:
> On Wed, Jul 28, 2010 at 05:41:47PM +0200, Michal Marek wrote:
>> (adding Nir, who wrote nconfig, to CC).
>>
>> On 28.7.2010 14:17, trapDoor wrote:
>> > On Wed, Jul 28, 2010 at 12:02 PM, Michal Marek <[email protected]> wrote:
>> >> respectively). Unfortunatelly, there doesn't seem to be a way to
>> >> highlight single letters in an ncurses menu, hence the strange
>> >> cApitalization :(.
>> >
>> > I wonder how is this resolved in other console applications such as
>> > Midnight Commander. Does it have its own implementation for
>> > highlighting characters (it uses coloured mnemonics e.g. in menus) or
>> > it's based on some common Linux/Unix library which perhaps could be
>> > used for the nconfig as well?
>>
>> The problem is that nconfig uses the higher-level ncurses libraries
>> libmenu and libpanel, which make some things easier, but aren't as
>> flexible as using ncurses directly. Patches are welcome of course :-).

The problem does arise from the use of libmenue, which gives you no
control over the rendering of menue items. You can not specify any
attributes for specific menu item's letters.

> The capitilazing is the main reason why I have
> not recommended nconfig to be default.
>
> And I doubt the shortcut letters are used that often.
> One idea could be to drop the shortcut for the individual menus.

I almost never use them and removing them will have the benefit of
cleaning the code.

A different approach may be to reimplement libmenu's logic (recreating
the old lxdialog code), but avoiding this was part of the reason for
writing nconfig....

In any way, I am happy people are starting to use nconfig.
Cheers.

2010-07-29 06:20:22

by trapDoor

[permalink] [raw]
Subject: Re: Small typo in kernel [current source from git] .config option

Hello,

On Thu, Jul 29, 2010 at 6:04 AM, Nir Tzachar <[email protected]> wrote:
> Hello.
>
> On Wed, Jul 28, 2010 at 9:55 PM, Sam Ravnborg <[email protected]> wrote:
>> On Wed, Jul 28, 2010 at 05:41:47PM +0200, Michal Marek wrote:
>>> (adding Nir, who wrote nconfig, to CC).
>>>
>>> On 28.7.2010 14:17, trapDoor wrote:
>>> > On Wed, Jul 28, 2010 at 12:02 PM, Michal Marek <[email protected]> wrote:
>>> >> respectively). Unfortunatelly, there doesn't seem to be a way to
>>> >> highlight single letters in an ncurses menu, hence the strange
>>> >> cApitalization :(.
>>> >
>>> > I wonder how is this resolved in other console applications such as
>>> > Midnight Commander. Does it have its own implementation for
>>> > highlighting characters (it uses coloured mnemonics e.g. in menus) or
>>> > it's based on some common Linux/Unix library which perhaps could be
>>> > used for the nconfig as well?
>>>
>>> The problem is that nconfig uses the higher-level ncurses libraries
>>> libmenu and libpanel, which make some things easier, but aren't as
>>> flexible as using ncurses directly. Patches are welcome of course :-).
>
> The problem does arise from the use of libmenue, which gives you no
> control over the rendering of menue items. You can not specify any
> attributes for specific menu item's letters.
>
>> The capitilazing is the main reason why I have
>> not recommended nconfig to be default.
>>
>> And I doubt the shortcut letters are used that often.
>> One idea could be to drop the shortcut for the individual menus.
>
> I almost never use them and removing them will have the benefit of
> cleaning the code.
>

Neither do I. And I don't know anyone who would find them very useful.
Anyway, if you compile kernel regularly, especially from git, you
usually keep the same .config from previous compilations, and just run
make oldconfig rather than any of the graphical tools. Unless you want
to do some major changes. But even then I don't think many people
would care about key shortcuts.

Only if they were going to be kept in nconfig I'd say that fixing the
capitalisation somehow would be a good thing to do. But if you are
going to remove them and thus make the fix possible and the code
cleaner - that suits me. I will still prefer the simple and clean
nconfig's interface over menuconfig with mnemonics.

> A different approach may be to reimplement libmenu's logic (recreating
> the old lxdialog code), but avoiding this was part of the reason for
> writing nconfig....
>
> In any way, I am happy people are starting to use nconfig.
> Cheers.
>

--
Thanks,
trapDoor

2010-07-29 08:05:51

by Michal Marek

[permalink] [raw]
Subject: Re: Small typo in kernel [current source from git] .config option

On 29.7.2010 08:20, trapDoor wrote:
> On Thu, Jul 29, 2010 at 6:04 AM, Nir Tzachar <[email protected]> wrote:
>> On Wed, Jul 28, 2010 at 9:55 PM, Sam Ravnborg <[email protected]> wrote:
>>> The capitilazing is the main reason why I have
>>> not recommended nconfig to be default.
>>>
>>> And I doubt the shortcut letters are used that often.
>>> One idea could be to drop the shortcut for the individual menus.
>>
>> I almost never use them and removing them will have the benefit of
>> cleaning the code.
>
> Neither do I. And I don't know anyone who would find them very useful.

I actually do. Unlike menuconfig, nconfig will jump to a menu item even
if it is not currently visible in the screen. Useful if you know the
option name but don't want to scroll down and look for it. But I would
accept a patch to remove the shortcut if there is no better way of
marking the shortcuts than the capital letters.

Michal

2010-07-29 10:33:52

by Nir Tzachar

[permalink] [raw]
Subject: Re: Small typo in kernel [current source from git] .config option

On Thu, Jul 29, 2010 at 11:05 AM, Michal Marek <[email protected]> wrote:
> On 29.7.2010 08:20, trapDoor wrote:
>> On Thu, Jul 29, 2010 at 6:04 AM, Nir Tzachar <[email protected]> wrote:
>>> On Wed, Jul 28, 2010 at 9:55 PM, Sam Ravnborg <[email protected]> wrote:
>>>> The capitilazing is the main reason why I have
>>>> not recommended nconfig to be default.
>>>>
>>>> And I doubt the shortcut letters are used that often.
>>>> One idea could be to drop the shortcut for the individual menus.
>>>
>>> I almost never use them and removing them will have the benefit of
>>> cleaning the code.
>>
>> Neither do I. And I don't know anyone who would find them very useful.
>
> I actually do. Unlike menuconfig, nconfig will jump to a menu item even
> if it is not currently visible in the screen. Useful if you know the
> option name but don't want to scroll down and look for it. But I would
> accept a patch to remove the shortcut if there is no better way of
> marking the shortcuts than the capital letters.
>
> Michal
>

There is a middle ground here: keep the shortcuts, but give no visual
indication (either capitalization or coloring).

What do you say? I actually like this, as I dislike the ugly
capitalization hack.

Cheers.

2010-07-29 14:19:23

by Randy Dunlap

[permalink] [raw]
Subject: Re: Small typo in kernel [current source from git] .config option

On Thu, 29 Jul 2010 13:33:49 +0300 Nir Tzachar wrote:

> On Thu, Jul 29, 2010 at 11:05 AM, Michal Marek <[email protected]> wrote:
> > On 29.7.2010 08:20, trapDoor wrote:
> >> On Thu, Jul 29, 2010 at 6:04 AM, Nir Tzachar <[email protected]> wrote:
> >>> On Wed, Jul 28, 2010 at 9:55 PM, Sam Ravnborg <[email protected]> wrote:
> >>>> The capitilazing is the main reason why I have
> >>>> not recommended nconfig to be default.
> >>>>
> >>>> And I doubt the shortcut letters are used that often.
> >>>> One idea could be to drop the shortcut for the individual menus.
> >>>
> >>> I almost never use them and removing them will have the benefit of
> >>> cleaning the code.
> >>
> >> Neither do I. And I don't know anyone who would find them very useful.
> >
> > I actually do. Unlike menuconfig, nconfig will jump to a menu item even
> > if it is not currently visible in the screen. Useful if you know the
> > option name but don't want to scroll down and look for it. But I would
> > accept a patch to remove the shortcut if there is no better way of
> > marking the shortcuts than the capital letters.
> >
> > Michal
> >
>
> There is a middle ground here: keep the shortcuts, but give no visual
> indication (either capitalization or coloring).

How do you tell libmenue what the shortcut character is?
Is that a different attribute/field/specifier?

If so, how about just adding the shortcut key in parentheses [or square
brackets] after its word or phrase? E.g.:

Hardware Error Device [A]


> What do you say? I actually like this, as I dislike the ugly
> capitalization hack.

It's not pretty and it can be confusing, but I would certainly prefer
to have some indicator for the shortcuts.

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

2010-07-29 19:50:56

by Nir Tzachar

[permalink] [raw]
Subject: Re: Small typo in kernel [current source from git] .config option

On Thu, Jul 29, 2010 at 5:19 PM, Randy Dunlap <[email protected]> wrote:
> On Thu, 29 Jul 2010 13:33:49 +0300 Nir Tzachar wrote:
>
>> On Thu, Jul 29, 2010 at 11:05 AM, Michal Marek <[email protected]> wrote:
>> > On 29.7.2010 08:20, trapDoor wrote:
>> >> On Thu, Jul 29, 2010 at 6:04 AM, Nir Tzachar <[email protected]> wrote:
>> >>> On Wed, Jul 28, 2010 at 9:55 PM, Sam Ravnborg <[email protected]> wrote:
>> >>>> The capitilazing is the main reason why I have
>> >>>> not recommended nconfig to be default.
>> >>>>
>> >>>> And I doubt the shortcut letters are used that often.
>> >>>> One idea could be to drop the shortcut for the individual menus.
>> >>>
>> >>> I almost never use them and removing them will have the benefit of
>> >>> cleaning the code.
>> >>
>> >> Neither do I. And I don't know anyone who would find them very useful.
>> >
>> > I actually do. Unlike menuconfig, nconfig will jump to a menu item even
>> > if it is not currently visible in the screen. Useful if you know the
>> > option name but don't want to scroll down and look for it. But I would
>> > accept a patch to remove the shortcut if there is no better way of
>> > marking the shortcuts than the capital letters.
>> >
>> > Michal
>> >
>>
>> There is a middle ground here: keep the shortcuts, but give no visual
>> indication (either capitalization or coloring).
>
> How do you tell libmenue what the shortcut character is?
> Is that a different attribute/field/specifier?

You can feed ascii chars to the menu_driver, which then employs a
pattern match to jump to a matched item. However, I have not used this
logic as I needed to implement the m,y,n etc. shortcuts.

Coming to think about it, it might be possible to use the built-in
pattern matching of libmenu by first pressing the '/' key, and then
feeding consecutive key presses to the menu_driver routine. I'll try
to come up with a patch.

> If so, how about just adding the shortcut key in parentheses [or square
> brackets] after its word or phrase? ?E.g.:
>
> ? ? ? ?Hardware Error Device [A]
>

This might be a problem, as there are options which are longer than
the width of a regular terminal..


Cheers.

2010-08-01 07:20:41

by Nir Tzachar

[permalink] [raw]
Subject: [PATCH] nconfig: add search support

From: Nir Tzachar <[email protected]>

Remove the old hotkeys feature, and replace by a regular string search.
>From nconfig help:

Pressing '/' triggers search mode. nconfig does regular string match, case
insensitive, starting at the begining of each menu line.
Pressing Enter highlights the next match, Backspace removes one character from
the match string. Pressing either '/' again or ESC exits search mode.

Rebind the '/' key (which allowed to search for symbols) to F8

Signed-off-by: Nir Tzachar <[email protected]>
---
scripts/kconfig/nconf.c | 261 ++++++++++++++++++++++-------------------------
scripts/kconfig/nconf.h | 3 +-
2 files changed, 125 insertions(+), 139 deletions(-)

diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 762caf8..2a7cb37 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -41,9 +41,12 @@ static const char nconf_readme[] = N_(
" pressing <Enter> of <right-arrow>. Use <Esc> or <left-arrow> to go back.\n"
" Submenus are designated by \"--->\".\n"
"\n"
-" Shortcut: Press the option's highlighted letter (hotkey).\n"
-" Pressing a hotkey more than once will sequence\n"
-" through all visible items which use that hotkey.\n"
+" Searching: pressing '/' triggers search mode. nconfig does a\n"
+" regular string match, case insensitive, starting at\n"
+" the begining of each menu line.\n"
+" Pressing Enter highlights the next match, Backspace\n"
+" removes one character from the match string.\n"
+" Pressing either '/' again or ESC exits search mode.\n"
"\n"
" You may also use the <PAGE UP> and <PAGE DOWN> keys to scroll\n"
" unseen options into view.\n"
@@ -252,7 +255,6 @@ struct mitem {
char str[256];
char tag;
void *usrptr;
- int is_hot;
int is_visible;
};

@@ -275,14 +277,6 @@ static int items_num;
static int global_exit;
/* the currently selected button */
const char *current_instructions = menu_instructions;
-/* this array is used to implement hot keys. it is updated in item_make and
- * resetted in clean_items. It would be better to use a hash, but lets keep it
- * simple... */
-#define MAX_SAME_KEY MAX_MENU_ITEMS
-struct {
- int count;
- int ptrs[MAX_MENU_ITEMS];
-} hotkeys[1<<(sizeof(char)*8)];

static void conf(struct menu *menu);
static void conf_choice(struct menu *menu);
@@ -292,6 +286,7 @@ static void conf_save(void);
static void show_help(struct menu *menu);
static int do_exit(void);
static void setup_windows(void);
+static void search_conf(void);

typedef void (*function_key_handler_t)(int *key, struct menu *menu);
static void handle_f1(int *key, struct menu *current_item);
@@ -302,6 +297,7 @@ static void handle_f5(int *key, struct menu *current_item);
static void handle_f6(int *key, struct menu *current_item);
static void handle_f7(int *key, struct menu *current_item);
static void handle_f8(int *key, struct menu *current_item);
+static void handle_f9(int *key, struct menu *current_item);

struct function_keys {
const char *key_str;
@@ -310,7 +306,7 @@ struct function_keys {
function_key_handler_t handler;
};

-static const int function_keys_num = 8;
+static const int function_keys_num = 9;
struct function_keys function_keys[] = {
{
.key_str = "F1",
@@ -356,9 +352,15 @@ struct function_keys function_keys[] = {
},
{
.key_str = "F8",
+ .func = "search symbol",
+ .key = F_SEARCH,
+ .handler = handle_f8,
+ },
+ {
+ .key_str = "F9",
.func = "Exit",
.key = F_EXIT,
- .handler = handle_f8,
+ .handler = handle_f9,
},
};

@@ -444,9 +446,16 @@ static void handle_f7(int *key, struct menu *current_item)
return;
}

-/* exit */
+/* search */
static void handle_f8(int *key, struct menu *current_item)
{
+ search_conf();
+ return;
+}
+
+/* exit */
+static void handle_f9(int *key, struct menu *current_item)
+{
do_exit();
return;
}
@@ -479,110 +488,45 @@ static void clean_items(void)
free_item(curses_menu_items[i]);
bzero(curses_menu_items, sizeof(curses_menu_items));
bzero(k_menu_items, sizeof(k_menu_items));
- bzero(hotkeys, sizeof(hotkeys));
items_num = 0;
}

-/* return the index of the next hot item, or -1 if no such item exists */
-static int get_next_hot(int c)
-{
- static int hot_index;
- static int hot_char;
+typedef enum {FIND_NEW_MATCH, FIND_NEXT_MATCH, FIND_NEXT_MATCH_INC} match_f;

- if (c < 0 || c > 255 || hotkeys[c].count <= 0)
- return -1;
-
- if (hot_char == c) {
- hot_index = (hot_index+1)%hotkeys[c].count;
- return hotkeys[c].ptrs[hot_index];
- } else {
- hot_char = c;
- hot_index = 0;
- return hotkeys[c].ptrs[0];
- }
-}
-
-/* can the char c be a hot key? no, if c is a common shortcut used elsewhere */
-static int canbhot(char c)
+/* return the index of the matched item, or -1 if no such item exists */
+static int get_mext_match(const char *match_str, match_f flag)
{
- c = tolower(c);
- return isalnum(c) && c != 'y' && c != 'm' && c != 'h' &&
- c != 'n' && c != '?';
-}
-
-/* check if str already contains a hot key. */
-static int is_hot(int index)
-{
- return k_menu_items[index].is_hot;
-}
-
-/* find the first possible hot key, and mark it.
- * index is the index of the item in the menu
- * return 0 on success*/
-static int make_hot(char *dest, int len, const char *org, int index)
-{
- int position = -1;
- int i;
- int tmp;
- int c;
- int org_len = strlen(org);
-
- if (org == NULL || is_hot(index))
- return 1;
-
- /* make sure not to make hot keys out of markers.
- * find where to start looking for a hot key
- */
- i = 0;
- /* skip white space */
- while (i < org_len && org[i] == ' ')
- i++;
- if (i == org_len)
- return -1;
- /* if encountering '(' or '<' or '[', find the match and look from there
- **/
- if (org[i] == '[' || org[i] == '<' || org[i] == '(') {
- i++;
- for (; i < org_len; i++)
- if (org[i] == ']' || org[i] == '>' || org[i] == ')')
- break;
- }
- if (i == org_len)
- return -1;
- for (; i < org_len; i++) {
- if (canbhot(org[i]) && org[i-1] != '<' && org[i-1] != '(') {
- position = i;
- break;
+ static int last_match;
+
+ if (flag == FIND_NEW_MATCH)
+ last_match = 0;
+ else if (flag == FIND_NEXT_MATCH_INC)
+ ++last_match;
+
+ int index;
+ for (index = last_match; index < items_num; ++index) {
+ char *non_space = k_menu_items[index].str;
+ /* skip the leading 4 bytes, as they are noise. */
+ non_space += 4;
+ /* and any white space from indentation */
+ while (*non_space != '\0' && isblank(*non_space))
+ ++non_space;
+ if (strncasecmp(match_str, non_space, strlen(match_str)) == 0) {
+ last_match = index;
+ return index;
}
}
- if (position == -1)
- return 1;

- /* ok, char at org[position] should be a hot key to this item */
- c = tolower(org[position]);
- tmp = hotkeys[c].count;
- hotkeys[c].ptrs[tmp] = index;
- hotkeys[c].count++;
- /*
- snprintf(dest, len, "%.*s(%c)%s", position, org, org[position],
- &org[position+1]);
- */
- /* make org[position] uppercase, and all leading letter small case */
- strncpy(dest, org, len);
- for (i = 0; i < position; i++)
- dest[i] = tolower(dest[i]);
- dest[position] = toupper(dest[position]);
- k_menu_items[index].is_hot = 1;
- return 0;
+ if (flag == FIND_NEXT_MATCH || flag == FIND_NEXT_MATCH_INC)
+ return get_mext_match(match_str, FIND_NEW_MATCH);
+ else
+ return -1;
}

-/* Make a new item. Add a hotkey mark in the first possible letter.
- * As ncurses does not allow any attributes inside menue item, we mark the
- * hot key as the first capitalized letter in the string */
+/* Make a new item. */
static void item_make(struct menu *menu, char tag, const char *fmt, ...)
{
va_list ap;
- char tmp_str[256];

if (items_num > MAX_MENU_ITEMS-1)
return;
@@ -597,16 +541,13 @@ static void item_make(struct menu *menu, char tag, const char *fmt, ...)
k_menu_items[items_num].is_visible = 1;

va_start(ap, fmt);
- vsnprintf(tmp_str, sizeof(tmp_str), fmt, ap);
- if (!k_menu_items[items_num].is_visible)
- memcpy(tmp_str, "XXX", 3);
+ vsnprintf(k_menu_items[items_num].str,
+ sizeof(k_menu_items[items_num].str),
+ fmt, ap);
va_end(ap);
- if (make_hot(
- k_menu_items[items_num].str,
- sizeof(k_menu_items[items_num].str), tmp_str, items_num) != 0)
- strncpy(k_menu_items[items_num].str,
- tmp_str,
- sizeof(k_menu_items[items_num].str));
+
+ if (!k_menu_items[items_num].is_visible)
+ memcpy(k_menu_items[items_num].str, "XXX", 3);

curses_menu_items[items_num] = new_item(
k_menu_items[items_num].str,
@@ -638,11 +579,9 @@ static void item_add_str(const char *fmt, ...)
va_end(ap);
snprintf(tmp_str, sizeof(tmp_str), "%s%s",
k_menu_items[index].str, new_str);
- if (make_hot(k_menu_items[index].str,
- sizeof(k_menu_items[index].str), tmp_str, index) != 0)
- strncpy(k_menu_items[index].str,
- tmp_str,
- sizeof(k_menu_items[index].str));
+ strncpy(k_menu_items[index].str,
+ tmp_str,
+ sizeof(k_menu_items[index].str));

free_item(curses_menu_items[index]);
curses_menu_items[index] = new_item(
@@ -1108,6 +1047,7 @@ static void conf(struct menu *menu)
int res;
int current_index = 0;
int last_top_row = 0;
+ int in_search = 0;

bzero(pattern, sizeof(pattern));

@@ -1150,22 +1090,44 @@ static void conf(struct menu *menu)
show_help((struct menu *) item_data());
break;
}
- if (res == 10 || res == 27 ||
+ if (res == '/' || (in_search && res == 27)) {
+ in_search = 1-in_search;
+ bzero(pattern, sizeof(pattern));
+ move(0, 0);
+ refresh();
+ clrtoeol();
+ } else if (in_search) {
+ char c = (char) res;
+ int tmp = -1;
+ if (isalnum(c)) {
+ pattern[strlen(pattern)] = c;
+ pattern[strlen(pattern)] = '\0';
+ tmp = get_mext_match(pattern,
+ FIND_NEXT_MATCH);
+ } else if (res == 10)
+ tmp = get_mext_match(pattern,
+ FIND_NEXT_MATCH_INC);
+ else if (res == KEY_BACKSPACE || res == 127) {
+ pattern[strlen(pattern)-1] = '\0';
+ tmp = get_mext_match(pattern,
+ FIND_NEXT_MATCH_INC);
+ }
+ if (tmp != -1)
+ center_item(tmp, &last_top_row);
+ } else if (res == 10 || res == 27 ||
res == 32 || res == 'n' || res == 'y' ||
res == KEY_LEFT || res == KEY_RIGHT ||
- res == 'm' || res == '/')
+ res == 'm')
break;
- else if (canbhot(res)) {
- /* check for hot keys: */
- int tmp = get_next_hot(res);
- if (tmp != -1)
- center_item(tmp, &last_top_row);
+ if (in_search) {
+ mvprintw(0, 0, "searching: %s", pattern);
+ clrtoeol();
}
refresh_all_windows(main_window);
}

refresh_all_windows(main_window);
- /* if ESC or left*/
+ /* if ESC or left*/
if (res == 27 || (menu != &rootmenu && res == KEY_LEFT))
break;

@@ -1233,9 +1195,6 @@ static void conf(struct menu *menu)
if (item_is_tag('t'))
sym_set_tristate_value(sym, mod);
break;
- case '/':
- search_conf();
- break;
}
}
}
@@ -1260,12 +1219,14 @@ static void show_help(struct menu *menu)

static void conf_choice(struct menu *menu)
{
+ char pattern[256];
const char *prompt = _(menu_get_prompt(menu));
struct menu *child = 0;
struct symbol *active;
int selected_index = 0;
int last_top_row = 0;
int res, i = 0;
+ int in_search = 0;

active = sym_get_choice_value(menu->sym);
/* this is mostly duplicated from the conf() function. */
@@ -1321,14 +1282,37 @@ static void conf_choice(struct menu *menu)
show_help((struct menu *) item_data());
break;
}
- if (res == 10 || res == 27 || res == ' ' ||
- res == KEY_LEFT)
- break;
- else if (canbhot(res)) {
- /* check for hot keys: */
- int tmp = get_next_hot(res);
+ if (res == '/' || (in_search && res == 27)) {
+ in_search = 1-in_search;
+ bzero(pattern, sizeof(pattern));
+ move(0, 0);
+ refresh();
+ clrtoeol();
+ } else if (in_search) {
+ char c = (char) res;
+ int tmp = -1;
+ if (isalnum(c)) {
+ pattern[strlen(pattern)] = c;
+ pattern[strlen(pattern)] = '\0';
+ tmp = get_mext_match(pattern,
+ FIND_NEXT_MATCH);
+ } else if (res == 10)
+ tmp = get_mext_match(pattern,
+ FIND_NEXT_MATCH_INC);
+ else if (res == KEY_BACKSPACE || res == 127) {
+ pattern[strlen(pattern)-1] = '\0';
+ tmp = get_mext_match(pattern,
+ FIND_NEXT_MATCH_INC);
+ }
if (tmp != -1)
center_item(tmp, &last_top_row);
+ } else if (res == 10 || res == 27 || res == ' ' ||
+ res == KEY_LEFT){
+ break;
+ }
+ if (in_search) {
+ mvprintw(0, 0, "searching: %s", pattern);
+ clrtoeol();
}
refresh_all_windows(main_window);
}
@@ -1530,9 +1514,10 @@ int main(int ac, char **av)
/* set btns menu */
curses_menu = new_menu(curses_menu_items);
menu_opts_off(curses_menu, O_SHOWDESC);
- menu_opts_off(curses_menu, O_SHOWMATCH);
+ menu_opts_on(curses_menu, O_SHOWMATCH);
menu_opts_on(curses_menu, O_ONEVALUE);
menu_opts_on(curses_menu, O_NONCYCLIC);
+ menu_opts_on(curses_menu, O_IGNORECASE);
set_menu_mark(curses_menu, " ");
set_menu_fore(curses_menu, attributes[MAIN_MENU_FORE]);
set_menu_back(curses_menu, attributes[MAIN_MENU_BACK]);
diff --git a/scripts/kconfig/nconf.h b/scripts/kconfig/nconf.h
index fb42966..58fbda8 100644
--- a/scripts/kconfig/nconf.h
+++ b/scripts/kconfig/nconf.h
@@ -69,7 +69,8 @@ typedef enum {
F_BACK = 5,
F_SAVE = 6,
F_LOAD = 7,
- F_EXIT = 8
+ F_SEARCH = 8,
+ F_EXIT = 9,
} function_key;

void set_colors(void);
--
1.6.4.4

2010-08-01 09:29:04

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] nconfig: add search support

On Sun, Aug 01, 2010 at 10:20:33AM +0300, [email protected] wrote:
> From: Nir Tzachar <[email protected]>
>
> Remove the old hotkeys feature, and replace by a regular string search.
> >From nconfig help:
>
> Pressing '/' triggers search mode. nconfig does regular string match, case
> insensitive, starting at the begining of each menu line.
> Pressing Enter highlights the next match, Backspace removes one character from
> the match string. Pressing either '/' again or ESC exits search mode.
>
> Rebind the '/' key (which allowed to search for symbols) to F8
>
> Signed-off-by: Nir Tzachar <[email protected]>

Got following warning when I build:
scripts/kconfig/nconf.c: In function ât_mext_matchâscripts/kconfig/nconf.c:512: warning: implicit declaration of function âblankâ

The searching works great! And good to see the capitilazation gone.

But it is ackward that I have to press ESC to leave searcing.

I can navigate up/down, but entering a menu should terminate search.
Also pressing enter (to select a symbol) should terminate search.

I also tried to do first search using "/", then search for a symbol using F8.
Again search was not terminated when I came back.

There is likely much more cases...

Sam

2010-08-01 10:21:51

by Nir Tzachar

[permalink] [raw]
Subject: Re: [PATCH] nconfig: add search support

On Sun, Aug 1, 2010 at 12:28 PM, Sam Ravnborg <[email protected]> wrote:
> On Sun, Aug 01, 2010 at 10:20:33AM +0300, [email protected] wrote:
>> From: Nir Tzachar <[email protected]>
>>
>> Remove the old hotkeys feature, and replace by a regular string search.
>> >From nconfig help:
>>
>> Pressing '/' triggers search mode. nconfig does regular string match, case
>> insensitive, starting at the begining of each menu line.
>> Pressing Enter highlights the next match, Backspace removes one character from
>> the match string. Pressing either '/' again or ESC exits search mode.
>>
>> Rebind the '/' key (which allowed to search for symbols) to F8
>>
>> Signed-off-by: Nir Tzachar <[email protected]>
>
> Got following warning when I build:
> scripts/kconfig/nconf.c: In function ?t_mext_match?scripts/kconfig/nconf.c:512: warning: implicit declaration of function ?blank?
>
> The searching works great! And good to see the capitilazation gone.

Perhaps now we can make nconfig the default?? ;)

> But it is ackward that I have to press ESC to leave searcing.
>
> I can navigate up/down, but entering a menu should terminate search.
> Also pressing enter (to select a symbol) should terminate search.
>
> I also tried to do first search using "/", then search for a symbol using F8.
> Again search was not terminated when I came back.
>
> There is likely much more cases...

Sure, but we need to come up with a good description of the behavior
we desire. As we did not discuss it, I just went ahead and implemented
a POC.

As to your input, the main problem with terminating a search is how to
still allow iteration between possible matches. We can let Return
terminate the search and use a different key (say, 'p') to navigate to
the next match. However, I dislike this idea, but if no one can come
up with something prettier, I'll implement it.

> ? ? ? ?Sam

Cheers.

2010-08-01 11:49:19

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] nconfig: add search support

> Sure, but we need to come up with a good description of the behavior
> we desire. As we did not discuss it, I just went ahead and implemented
> a POC.

How about something like this:

In search mode:
- all normal letters / numbers are added to the search criteria

arrow-up/arrow-down move cursor to prev/next match
ESC terminate search mode

All other keys terminate search and take usual action
This include space, arrow-left/right, page-up/down, F*, enter, etc.

Sam

2010-08-02 16:33:06

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] nconfig: add search support

On Sun, 1 Aug 2010 10:20:33 +0300 [email protected] wrote:

> From: Nir Tzachar <[email protected]>
>
> Remove the old hotkeys feature, and replace by a regular string search.
> From nconfig help:
>
> Pressing '/' triggers search mode. nconfig does regular string match, case
> insensitive, starting at the begining of each menu line.

What is a "regular string match"?
That's not the same as a regular expression match, right?

Maybe it's a "simple string match"?

> Pressing Enter highlights the next match, Backspace removes one character from
> the match string. Pressing either '/' again or ESC exits search mode.
>
> Rebind the '/' key (which allowed to search for symbols) to F8
>
> Signed-off-by: Nir Tzachar <[email protected]>
> ---
> scripts/kconfig/nconf.c | 261 ++++++++++++++++++++++-------------------------
> scripts/kconfig/nconf.h | 3 +-
> 2 files changed, 125 insertions(+), 139 deletions(-)
>
> diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
> index 762caf8..2a7cb37 100644
> --- a/scripts/kconfig/nconf.c
> +++ b/scripts/kconfig/nconf.c
> @@ -41,9 +41,12 @@ static const char nconf_readme[] = N_(
> " pressing <Enter> of <right-arrow>. Use <Esc> or <left-arrow> to go back.\n"
> " Submenus are designated by \"--->\".\n"
> "\n"
> -" Shortcut: Press the option's highlighted letter (hotkey).\n"
> -" Pressing a hotkey more than once will sequence\n"
> -" through all visible items which use that hotkey.\n"
> +" Searching: pressing '/' triggers search mode. nconfig does a\n"
> +" regular string match, case insensitive, starting at\n"
> +" the begining of each menu line.\n"
> +" Pressing Enter highlights the next match, Backspace\n"
> +" removes one character from the match string.\n"
> +" Pressing either '/' again or ESC exits search mode.\n"
> "\n"
> " You may also use the <PAGE UP> and <PAGE DOWN> keys to scroll\n"
> " unseen options into view.\n"


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

2010-08-02 21:22:31

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] nconfig: add search support

On Sun, August 1, 2010 12:20 am, [email protected] wrote:
> From: Nir Tzachar <[email protected]>
>
>
> Remove the old hotkeys feature, and replace by a regular string search.
> From nconfig help:
>
>
> Pressing '/' triggers search mode. nconfig does regular string match,
> case insensitive, starting at the begining of each menu line. Pressing
> Enter highlights the next match, Backspace removes one character from
> the match string. Pressing either '/' again or ESC exits search mode.
>
> Rebind the '/' key (which allowed to search for symbols) to F8
>
>
> Signed-off-by: Nir Tzachar <[email protected]>
> ---
> scripts/kconfig/nconf.c | 261
> ++++++++++++++++++++++-------------------------
> scripts/kconfig/nconf.h | 3 +- 2 files changed, 125 insertions(+), 139
> deletions(-)


A few other comments:

These help text lines should end with a period:

Pressing SpaceBar toggles between the above options

<Esc> always leaves the current window


The F-key help along the bottom line should be forced to fit
into 80 columns. F9 (exit) especially should be visible.


This F3 should be F5:

Press <Esc>, <F3> or <left-arrow> to go back one menu,


How about we make nconfig the default in linux-next
so that it will be used more?

2010-08-03 04:37:48

by Nir Tzachar

[permalink] [raw]
Subject: Re: [PATCH] nconfig: add search support

On Mon, Aug 2, 2010 at 7:32 PM, Randy Dunlap <[email protected]> wrote:
> On Sun, ?1 Aug 2010 10:20:33 +0300 [email protected] wrote:
>
>> From: Nir Tzachar <[email protected]>
>>
>> Remove the old hotkeys feature, and replace by a regular string search.
>> From nconfig help:
>>
>> Pressing '/' triggers search mode. nconfig does regular string match, case
>> insensitive, starting at the begining of each menu line.
>
> What is a "regular string match"?
> That's not the same as a regular expression match, right?
>
> Maybe it's a "simple string match"?

strcasecmp. I've considered using regex or fnmatch, but it seemed an overkill.

>> Pressing Enter highlights the next match, Backspace removes one character from
>> the match string. Pressing either '/' again or ESC exits search mode.
>>
>> Rebind the '/' key (which allowed to search for symbols) to F8
>>
>> Signed-off-by: Nir Tzachar <[email protected]>
>> ---
>> ?scripts/kconfig/nconf.c | ?261 ++++++++++++++++++++++-------------------------
>> ?scripts/kconfig/nconf.h | ? ?3 +-
>> ?2 files changed, 125 insertions(+), 139 deletions(-)
>>
>> diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
>> index 762caf8..2a7cb37 100644
>> --- a/scripts/kconfig/nconf.c
>> +++ b/scripts/kconfig/nconf.c
>> @@ -41,9 +41,12 @@ static const char nconf_readme[] = N_(
>> ?" ? pressing <Enter> of <right-arrow>. Use <Esc> or <left-arrow> to go back.\n"
>> ?" ? Submenus are designated by \"--->\".\n"
>> ?"\n"
>> -" ? Shortcut: Press the option's highlighted letter (hotkey).\n"
>> -" ? ? ? ? ? ? Pressing a hotkey more than once will sequence\n"
>> -" ? ? ? ? ? ? through all visible items which use that hotkey.\n"
>> +" ? Searching: pressing '/' triggers search mode. nconfig does a\n"
>> +" ? ? ? ? ? ? ?regular string match, case insensitive, starting at\n"
>> +" ? ? ? ? ? ? ?the begining of each menu line.\n"
>> +" ? ? ? ? ? ? ?Pressing Enter highlights the next match, Backspace\n"
>> +" ? ? ? ? ? ? ?removes one character from the match string.\n"
>> +" ? ? ? ? ? ? ?Pressing either '/' again or ESC exits search mode.\n"
>> ?"\n"
>> ?" ? You may also use the <PAGE UP> and <PAGE DOWN> keys to scroll\n"
>> ?" ? unseen options into view.\n"
>
>
> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>

2010-08-03 04:41:27

by Nir Tzachar

[permalink] [raw]
Subject: Re: [PATCH] nconfig: add search support

On Tue, Aug 3, 2010 at 12:22 AM, <[email protected]> wrote:
> On Sun, August 1, 2010 12:20 am, [email protected] wrote:
>> From: Nir Tzachar <[email protected]>
>>
>>
>> Remove the old hotkeys feature, and replace by a regular string search.
>> From nconfig help:
>>
>>
>> Pressing '/' triggers search mode. nconfig does regular string match,
>> case insensitive, starting at the begining of each menu line. Pressing
>> Enter highlights the next match, Backspace removes one character from
>> the match string. Pressing either '/' again or ESC exits search mode.
>>
>> Rebind the '/' key (which allowed to search for symbols) to F8
>>
>>
>> Signed-off-by: Nir Tzachar <[email protected]>
>> ---
>> scripts/kconfig/nconf.c | ?261
>> ++++++++++++++++++++++-------------------------
>> scripts/kconfig/nconf.h | ? ?3 +- 2 files changed, 125 insertions(+), 139
>> deletions(-)
>
>
> A few other comments:
>
> These help text lines should end with a period:
>
> ?Pressing SpaceBar toggles between the above options
>
> ?<Esc> always leaves the current window
>

Sure.

> The F-key help along the bottom line should be forced to fit
> into 80 columns. ?F9 (exit) especially should be visible.

They were, however I think that the symbol search (F-8) kind of ruind
it. I'll shorten its name.

>
> This F3 should be F5:
>
> ?Press <Esc>, <F3> or <left-arrow> to go back one menu,

10x.

>
> How about we make nconfig the default in linux-next
> so that it will be used more?

I would love to see that. Let me finalize a patch which properly
supports searching (based on Sam's input) so as to remove the current
upper case ugliness.

Cheers.

2010-08-03 12:28:52

by Nir Tzachar

[permalink] [raw]
Subject: [PATCH] nconfig: add search support

From: Nir Tzachar <[email protected]>

Remove the old hotkeys feature, and replace by a regular string search.
Behaviour of search is as advised by Sam.

>From nconfig help:

Searching: pressing '/' triggers search mode. nconfig performs a
regular string compare, case insensitive, starting at
the beginning of each menu line.\n"
Pressing the up/down keys highlights the previous/next
matching item. Backspace removes one character from the
match string. Pressing either '/' again or ESC exits\
search mode. All other keys behave normally.

Miscellaneous other changes (including Rundy's Justin's input).

Signed-off-by: Nir Tzachar <[email protected]>
---
scripts/kconfig/nconf.c | 350 +++++++++++++++++++++++++------------------
scripts/kconfig/nconf.gui.c | 20 ++--
scripts/kconfig/nconf.h | 3 +-
3 files changed, 213 insertions(+), 160 deletions(-)

diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 762caf8..9cda16b 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -8,6 +8,7 @@
#define LKC_DIRECT_LINK
#include "lkc.h"
#include "nconf.h"
+#include <ctype.h>

static const char nconf_readme[] = N_(
"Overview\n"
@@ -23,7 +24,7 @@ static const char nconf_readme[] = 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"
-" XXX cannot be selected. use Symbol Info to find out why,\n"
+" XXX cannot be selected. Use Symbol Info to find out why,\n"
"while *, M or whitespace inside braces means to build in, build as\n"
"a module or to exclude the feature respectively.\n"
"\n"
@@ -41,9 +42,13 @@ static const char nconf_readme[] = N_(
" pressing <Enter> of <right-arrow>. Use <Esc> or <left-arrow> to go back.\n"
" Submenus are designated by \"--->\".\n"
"\n"
-" Shortcut: Press the option's highlighted letter (hotkey).\n"
-" Pressing a hotkey more than once will sequence\n"
-" through all visible items which use that hotkey.\n"
+" Searching: pressing '/' triggers search mode. nconfig performs a\n"
+" regular string compare, case insensitive, starting at\n"
+" the beginning of each menu line.\n"
+" Pressing the up/down keys highlights the previous/next\n"
+" matching item. Backspace removes one character from the\n"
+" match string. Pressing either '/' again or ESC exits\n"
+" search mode. All other keys behave normally.\n"
"\n"
" You may also use the <PAGE UP> and <PAGE DOWN> keys to scroll\n"
" unseen options into view.\n"
@@ -141,21 +146,21 @@ menu_no_f_instructions[] = N_(
" <Enter> or <right-arrow> selects submenus --->.\n"
" Capital Letters are hotkeys.\n"
" Pressing <Y> includes, <N> excludes, <M> modularizes features.\n"
-" Pressing SpaceBar toggles between the above options\n"
+" Pressing SpaceBar toggles between the above options.\n"
" Press <Esc> or <left-arrow> to go back one menu,\n"
" <?> or <h> for Help, </> for Search.\n"
-" <1> is interchangable with <F1>, <2> with <F2>, etc.\n"
+" <1> is interchangeable with <F1>, <2> with <F2>, etc.\n"
" Legend: [*] built-in [ ] excluded <M> module < > module capable.\n"
-" <Esc> always leaves the current window\n"),
+" <Esc> always leaves the current window.\n"),
menu_instructions[] = N_(
" Arrow keys navigate the menu.\n"
" <Enter> or <right-arrow> selects submenus --->.\n"
" Capital Letters are hotkeys.\n"
" Pressing <Y> includes, <N> excludes, <M> modularizes features.\n"
" Pressing SpaceBar toggles between the above options\n"
-" Press <Esc>, <F3> or <left-arrow> to go back one menu, \n"
+" Press <Esc>, <F5> or <left-arrow> to go back one menu,\n"
" <?>, <F1> or <h> for Help, </> for Search.\n"
-" <1> is interchangable with <F1>, <2> with <F2>, etc.\n"
+" <1> is interchangeable with <F1>, <2> with <F2>, etc.\n"
" Legend: [*] built-in [ ] excluded <M> module < > module capable.\n"
" <Esc> always leaves the current window\n"),
radiolist_instructions[] = N_(
@@ -252,7 +257,6 @@ struct mitem {
char str[256];
char tag;
void *usrptr;
- int is_hot;
int is_visible;
};

@@ -275,14 +279,6 @@ static int items_num;
static int global_exit;
/* the currently selected button */
const char *current_instructions = menu_instructions;
-/* this array is used to implement hot keys. it is updated in item_make and
- * resetted in clean_items. It would be better to use a hash, but lets keep it
- * simple... */
-#define MAX_SAME_KEY MAX_MENU_ITEMS
-struct {
- int count;
- int ptrs[MAX_MENU_ITEMS];
-} hotkeys[1<<(sizeof(char)*8)];

static void conf(struct menu *menu);
static void conf_choice(struct menu *menu);
@@ -292,6 +288,7 @@ static void conf_save(void);
static void show_help(struct menu *menu);
static int do_exit(void);
static void setup_windows(void);
+static void search_conf(void);

typedef void (*function_key_handler_t)(int *key, struct menu *menu);
static void handle_f1(int *key, struct menu *current_item);
@@ -302,6 +299,7 @@ static void handle_f5(int *key, struct menu *current_item);
static void handle_f6(int *key, struct menu *current_item);
static void handle_f7(int *key, struct menu *current_item);
static void handle_f8(int *key, struct menu *current_item);
+static void handle_f9(int *key, struct menu *current_item);

struct function_keys {
const char *key_str;
@@ -310,7 +308,7 @@ struct function_keys {
function_key_handler_t handler;
};

-static const int function_keys_num = 8;
+static const int function_keys_num = 9;
struct function_keys function_keys[] = {
{
.key_str = "F1",
@@ -320,13 +318,13 @@ struct function_keys function_keys[] = {
},
{
.key_str = "F2",
- .func = "Symbol Info",
+ .func = "Sym Info",
.key = F_SYMBOL,
.handler = handle_f2,
},
{
.key_str = "F3",
- .func = "Instructions",
+ .func = "Insts",
.key = F_INSTS,
.handler = handle_f3,
},
@@ -356,9 +354,15 @@ struct function_keys function_keys[] = {
},
{
.key_str = "F8",
+ .func = "Sym Search",
+ .key = F_SEARCH,
+ .handler = handle_f8,
+ },
+ {
+ .key_str = "F9",
.func = "Exit",
.key = F_EXIT,
- .handler = handle_f8,
+ .handler = handle_f9,
},
};

@@ -444,9 +448,16 @@ static void handle_f7(int *key, struct menu *current_item)
return;
}

-/* exit */
+/* search */
static void handle_f8(int *key, struct menu *current_item)
{
+ search_conf();
+ return;
+}
+
+/* exit */
+static void handle_f9(int *key, struct menu *current_item)
+{
do_exit();
return;
}
@@ -479,110 +490,50 @@ static void clean_items(void)
free_item(curses_menu_items[i]);
bzero(curses_menu_items, sizeof(curses_menu_items));
bzero(k_menu_items, sizeof(k_menu_items));
- bzero(hotkeys, sizeof(hotkeys));
items_num = 0;
}

-/* return the index of the next hot item, or -1 if no such item exists */
-static int get_next_hot(int c)
-{
- static int hot_index;
- static int hot_char;
-
- if (c < 0 || c > 255 || hotkeys[c].count <= 0)
- return -1;
-
- if (hot_char == c) {
- hot_index = (hot_index+1)%hotkeys[c].count;
- return hotkeys[c].ptrs[hot_index];
- } else {
- hot_char = c;
- hot_index = 0;
- return hotkeys[c].ptrs[0];
- }
-}
-
-/* can the char c be a hot key? no, if c is a common shortcut used elsewhere */
-static int canbhot(char c)
-{
- c = tolower(c);
- return isalnum(c) && c != 'y' && c != 'm' && c != 'h' &&
- c != 'n' && c != '?';
-}
+typedef enum {FIND_NEW_MATCH, FIND_NEXT_MATCH, FIND_NEXT_MATCH_INC,
+ FIND_NEXT_MATCH_DEC} match_f;

-/* check if str already contains a hot key. */
-static int is_hot(int index)
+/* return the index of the matched item, or -1 if no such item exists */
+static int get_mext_match(const char *match_str, int last_match, match_f flag)
{
- return k_menu_items[index].is_hot;
-}
-
-/* find the first possible hot key, and mark it.
- * index is the index of the item in the menu
- * return 0 on success*/
-static int make_hot(char *dest, int len, const char *org, int index)
-{
- int position = -1;
- int i;
- int tmp;
- int c;
- int org_len = strlen(org);
-
- if (org == NULL || is_hot(index))
- return 1;
-
- /* make sure not to make hot keys out of markers.
- * find where to start looking for a hot key
- */
- i = 0;
- /* skip white space */
- while (i < org_len && org[i] == ' ')
- i++;
- if (i == org_len)
- return -1;
- /* if encountering '(' or '<' or '[', find the match and look from there
- **/
- if (org[i] == '[' || org[i] == '<' || org[i] == '(') {
- i++;
- for (; i < org_len; i++)
- if (org[i] == ']' || org[i] == '>' || org[i] == ')')
- break;
- }
- if (i == org_len)
- return -1;
- for (; i < org_len; i++) {
- if (canbhot(org[i]) && org[i-1] != '<' && org[i-1] != '(') {
- position = i;
- break;
+ int match_start = last_match;
+ int index;
+
+ if (flag == FIND_NEW_MATCH)
+ last_match = 0;
+ if (flag == FIND_NEXT_MATCH_INC)
+ ++last_match;
+ else if (flag == FIND_NEXT_MATCH_DEC)
+ --last_match;
+
+ index = last_match;
+ while (true) {
+ char *non_space = k_menu_items[index].str;
+ /* skip the leading 4 bytes, as they are noise. */
+ non_space += 4;
+ /* and any white space from indentation */
+ while (*non_space != '\0' && isblank(*non_space))
+ ++non_space;
+ if (strncasecmp(match_str, non_space, strlen(match_str)) == 0) {
+ return index;
}
+ if (flag == FIND_NEXT_MATCH_DEC || flag == FIND_NEXT_MATCH)
+ --index;
+ else
+ ++index;
+ index = (index + items_num) % items_num;
+ if (index == match_start)
+ return -1;
}
- if (position == -1)
- return 1;
-
- /* ok, char at org[position] should be a hot key to this item */
- c = tolower(org[position]);
- tmp = hotkeys[c].count;
- hotkeys[c].ptrs[tmp] = index;
- hotkeys[c].count++;
- /*
- snprintf(dest, len, "%.*s(%c)%s", position, org, org[position],
- &org[position+1]);
- */
- /* make org[position] uppercase, and all leading letter small case */
- strncpy(dest, org, len);
- for (i = 0; i < position; i++)
- dest[i] = tolower(dest[i]);
- dest[position] = toupper(dest[position]);
- k_menu_items[index].is_hot = 1;
- return 0;
}

-/* Make a new item. Add a hotkey mark in the first possible letter.
- * As ncurses does not allow any attributes inside menue item, we mark the
- * hot key as the first capitalized letter in the string */
+/* Make a new item. */
static void item_make(struct menu *menu, char tag, const char *fmt, ...)
{
va_list ap;
- char tmp_str[256];

if (items_num > MAX_MENU_ITEMS-1)
return;
@@ -597,16 +548,13 @@ static void item_make(struct menu *menu, char tag, const char *fmt, ...)
k_menu_items[items_num].is_visible = 1;

va_start(ap, fmt);
- vsnprintf(tmp_str, sizeof(tmp_str), fmt, ap);
- if (!k_menu_items[items_num].is_visible)
- memcpy(tmp_str, "XXX", 3);
+ vsnprintf(k_menu_items[items_num].str,
+ sizeof(k_menu_items[items_num].str),
+ fmt, ap);
va_end(ap);
- if (make_hot(
- k_menu_items[items_num].str,
- sizeof(k_menu_items[items_num].str), tmp_str, items_num) != 0)
- strncpy(k_menu_items[items_num].str,
- tmp_str,
- sizeof(k_menu_items[items_num].str));
+
+ if (!k_menu_items[items_num].is_visible)
+ memcpy(k_menu_items[items_num].str, "XXX", 3);

curses_menu_items[items_num] = new_item(
k_menu_items[items_num].str,
@@ -638,11 +586,9 @@ static void item_add_str(const char *fmt, ...)
va_end(ap);
snprintf(tmp_str, sizeof(tmp_str), "%s%s",
k_menu_items[index].str, new_str);
- if (make_hot(k_menu_items[index].str,
- sizeof(k_menu_items[index].str), tmp_str, index) != 0)
- strncpy(k_menu_items[index].str,
- tmp_str,
- sizeof(k_menu_items[index].str));
+ strncpy(k_menu_items[index].str,
+ tmp_str,
+ sizeof(k_menu_items[index].str));

free_item(curses_menu_items[index]);
curses_menu_items[index] = new_item(
@@ -1108,6 +1054,8 @@ static void conf(struct menu *menu)
int res;
int current_index = 0;
int last_top_row = 0;
+ int in_search = 0;
+ int last_match = 0;

bzero(pattern, sizeof(pattern));

@@ -1122,7 +1070,64 @@ static void conf(struct menu *menu)
_(menu_instructions),
current_index, &last_top_row);
keypad((menu_win(curses_menu)), TRUE);
- while (!global_exit && (res = wgetch(menu_win(curses_menu)))) {
+ while (!global_exit) {
+ if (in_search) {
+ mvprintw(0, 0, "searching: %s", pattern);
+ clrtoeol();
+ } else {
+ move(0, 0);
+ refresh();
+ clrtoeol();
+ }
+ refresh_all_windows(main_window);
+ res = wgetch(menu_win(curses_menu));
+ if (!res)
+ break;
+ if (res == '/' || (in_search && res == 27)) {
+ in_search = 1-in_search;
+ bzero(pattern, sizeof(pattern));
+ continue;
+ } else if (in_search) {
+ char c = (char) res;
+ int tmp = -1;
+ int terminate_search = 0;
+ if (isalnum(c)) {
+ pattern[strlen(pattern)] = c;
+ pattern[strlen(pattern)] = '\0';
+ tmp = get_mext_match(pattern,
+ last_match,
+ FIND_NEXT_MATCH);
+ } else if (res == KEY_DOWN)
+ tmp = get_mext_match(pattern,
+ last_match,
+ FIND_NEXT_MATCH_INC);
+ else if (res == KEY_UP)
+ tmp = get_mext_match(pattern,
+ last_match,
+ FIND_NEXT_MATCH_DEC);
+ else if (res == KEY_BACKSPACE || res == 127) {
+ pattern[strlen(pattern)-1] = '\0';
+ tmp = get_mext_match(pattern,
+ last_match,
+ FIND_NEXT_MATCH_INC);
+ } else
+ terminate_search = 1;
+
+ if (terminate_search) {
+ in_search = 1-in_search;
+ bzero(pattern, sizeof(pattern));
+ move(0, 0);
+ refresh();
+ clrtoeol();
+ /* fall through to normal mode */
+ } else {
+ if (tmp != -1) {
+ center_item(tmp, &last_top_row);
+ last_match = tmp;
+ }
+ continue;
+ }
+ }
if (process_special_keys(&res,
(struct menu *) item_data()))
break;
@@ -1153,19 +1158,13 @@ static void conf(struct menu *menu)
if (res == 10 || res == 27 ||
res == 32 || res == 'n' || res == 'y' ||
res == KEY_LEFT || res == KEY_RIGHT ||
- res == 'm' || res == '/')
+ res == 'm')
break;
- else if (canbhot(res)) {
- /* check for hot keys: */
- int tmp = get_next_hot(res);
- if (tmp != -1)
- center_item(tmp, &last_top_row);
- }
refresh_all_windows(main_window);
}

refresh_all_windows(main_window);
- /* if ESC or left*/
+ /* if ESC or left*/
if (res == 27 || (menu != &rootmenu && res == KEY_LEFT))
break;

@@ -1233,9 +1232,6 @@ static void conf(struct menu *menu)
if (item_is_tag('t'))
sym_set_tristate_value(sym, mod);
break;
- case '/':
- search_conf();
- break;
}
}
}
@@ -1260,12 +1256,15 @@ static void show_help(struct menu *menu)

static void conf_choice(struct menu *menu)
{
+ char pattern[256];
const char *prompt = _(menu_get_prompt(menu));
struct menu *child = 0;
struct symbol *active;
int selected_index = 0;
int last_top_row = 0;
int res, i = 0;
+ int in_search = 0;
+ int last_match = 0;

active = sym_get_choice_value(menu->sym);
/* this is mostly duplicated from the conf() function. */
@@ -1292,7 +1291,64 @@ static void conf_choice(struct menu *menu)
_(radiolist_instructions),
selected_index,
&last_top_row);
- while (!global_exit && (res = wgetch(menu_win(curses_menu)))) {
+ while (!global_exit) {
+ if (in_search) {
+ mvprintw(0, 0, "searching: %s", pattern);
+ clrtoeol();
+ } else {
+ move(0, 0);
+ refresh();
+ clrtoeol();
+ }
+ refresh_all_windows(main_window);
+ res = wgetch(menu_win(curses_menu));
+ if (!res)
+ break;
+ if (res == '/' || (in_search && res == 27)) {
+ in_search = 1-in_search;
+ bzero(pattern, sizeof(pattern));
+ continue;
+ } else if (in_search) {
+ char c = (char) res;
+ int tmp = -1;
+ int terminate_search = 0;
+ if (isalnum(c)) {
+ pattern[strlen(pattern)] = c;
+ pattern[strlen(pattern)] = '\0';
+ tmp = get_mext_match(pattern,
+ last_match,
+ FIND_NEXT_MATCH);
+ } else if (res == KEY_DOWN)
+ tmp = get_mext_match(pattern,
+ last_match,
+ FIND_NEXT_MATCH_INC);
+ else if (res == KEY_UP)
+ tmp = get_mext_match(pattern,
+ last_match,
+ FIND_NEXT_MATCH_DEC);
+ else if (res == KEY_BACKSPACE || res == 127) {
+ pattern[strlen(pattern)-1] = '\0';
+ tmp = get_mext_match(pattern,
+ last_match,
+ FIND_NEXT_MATCH_INC);
+ } else
+ terminate_search = 1;
+
+ if (terminate_search) {
+ in_search = 1-in_search;
+ bzero(pattern, sizeof(pattern));
+ move(0, 0);
+ refresh();
+ clrtoeol();
+ /* fall through to normal mode */
+ } else {
+ if (tmp != -1) {
+ center_item(tmp, &last_top_row);
+ last_match = tmp;
+ }
+ continue;
+ }
+ }
if (process_special_keys(
&res,
(struct menu *) item_data()))
@@ -1322,13 +1378,8 @@ static void conf_choice(struct menu *menu)
break;
}
if (res == 10 || res == 27 || res == ' ' ||
- res == KEY_LEFT)
+ res == KEY_LEFT){
break;
- else if (canbhot(res)) {
- /* check for hot keys: */
- int tmp = get_next_hot(res);
- if (tmp != -1)
- center_item(tmp, &last_top_row);
}
refresh_all_windows(main_window);
}
@@ -1530,9 +1581,10 @@ int main(int ac, char **av)
/* set btns menu */
curses_menu = new_menu(curses_menu_items);
menu_opts_off(curses_menu, O_SHOWDESC);
- menu_opts_off(curses_menu, O_SHOWMATCH);
+ menu_opts_on(curses_menu, O_SHOWMATCH);
menu_opts_on(curses_menu, O_ONEVALUE);
menu_opts_on(curses_menu, O_NONCYCLIC);
+ menu_opts_on(curses_menu, O_IGNORECASE);
set_menu_mark(curses_menu, " ");
set_menu_fore(curses_menu, attributes[MAIN_MENU_FORE]);
set_menu_back(curses_menu, attributes[MAIN_MENU_BACK]);
diff --git a/scripts/kconfig/nconf.gui.c b/scripts/kconfig/nconf.gui.c
index a9d9344..d963071 100644
--- a/scripts/kconfig/nconf.gui.c
+++ b/scripts/kconfig/nconf.gui.c
@@ -167,7 +167,7 @@ void print_in_middle(WINDOW *win,
length = strlen(string);
temp = (width - length) / 2;
x = startx + (int)temp;
- wattrset(win, color);
+ (void) wattrset(win, color);
mvwprintw(win, y, x, "%s", string);
refresh();
}
@@ -297,11 +297,11 @@ int btn_dialog(WINDOW *main_window, const char *msg, int btn_num, ...)
set_menu_fore(menu, attributes[DIALOG_MENU_FORE]);
set_menu_back(menu, attributes[DIALOG_MENU_BACK]);

- wattrset(win, attributes[DIALOG_BOX]);
+ (void) wattrset(win, attributes[DIALOG_BOX]);
box(win, 0, 0);

/* print message */
- wattrset(msg_win, attributes[DIALOG_TEXT]);
+ (void) wattrset(msg_win, attributes[DIALOG_TEXT]);
fill_window(msg_win, msg);

set_menu_win(menu, win);
@@ -392,16 +392,16 @@ int dialog_inputbox(WINDOW *main_window,
form_win = derwin(win, 1, prompt_width, prompt_lines+3, 2);
keypad(form_win, TRUE);

- wattrset(form_win, attributes[INPUT_FIELD]);
+ (void) wattrset(form_win, attributes[INPUT_FIELD]);

- wattrset(win, attributes[INPUT_BOX]);
+ (void) wattrset(win, attributes[INPUT_BOX]);
box(win, 0, 0);
- wattrset(win, attributes[INPUT_HEADING]);
+ (void) wattrset(win, attributes[INPUT_HEADING]);
if (title)
mvwprintw(win, 0, 3, "%s", title);

/* print message */
- wattrset(prompt_win, attributes[INPUT_TEXT]);
+ (void) wattrset(prompt_win, attributes[INPUT_TEXT]);
fill_window(prompt_win, prompt);

mvwprintw(form_win, 0, 0, "%*s", prompt_width, " ");
@@ -531,7 +531,7 @@ void show_scroll_win(WINDOW *main_window,

/* create the pad */
pad = newpad(total_lines+10, total_cols+10);
- wattrset(pad, attributes[SCROLLWIN_TEXT]);
+ (void) wattrset(pad, attributes[SCROLLWIN_TEXT]);
fill_window(pad, text);

win_lines = min(total_lines+4, LINES-2);
@@ -546,9 +546,9 @@ void show_scroll_win(WINDOW *main_window,
win = newwin(win_lines, win_cols, y, x);
keypad(win, TRUE);
/* show the help in the help window, and show the help panel */
- wattrset(win, attributes[SCROLLWIN_BOX]);
+ (void) wattrset(win, attributes[SCROLLWIN_BOX]);
box(win, 0, 0);
- wattrset(win, attributes[SCROLLWIN_HEADING]);
+ (void) wattrset(win, attributes[SCROLLWIN_HEADING]);
mvwprintw(win, 0, 3, " %s ", title);
panel = new_panel(win);

diff --git a/scripts/kconfig/nconf.h b/scripts/kconfig/nconf.h
index fb42966..58fbda8 100644
--- a/scripts/kconfig/nconf.h
+++ b/scripts/kconfig/nconf.h
@@ -69,7 +69,8 @@ typedef enum {
F_BACK = 5,
F_SAVE = 6,
F_LOAD = 7,
- F_EXIT = 8
+ F_SEARCH = 8,
+ F_EXIT = 9,
} function_key;

void set_colors(void);
--
1.6.4.4

2010-08-05 17:04:30

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] nconfig: add search support

On Tue, 3 Aug 2010 15:28:12 +0300 [email protected] wrote:

> From: Nir Tzachar <[email protected]>
>
> Remove the old hotkeys feature, and replace by a regular string search.
> Behaviour of search is as advised by Sam.
>
> From nconfig help:
>
> Searching: pressing '/' triggers search mode. nconfig performs a
> regular string compare, case insensitive, starting at

I would say: simple string compare
"regular" has no meaning (at least for me) there.

> the beginning of each menu line.\n"
> Pressing the up/down keys highlights the previous/next
> matching item. Backspace removes one character from the
> match string. Pressing either '/' again or ESC exits\
> search mode. All other keys behave normally.
>
> Miscellaneous other changes (including Rundy's Justin's input).

Yes, the bottom menu and the typos are fixed. Thanks.

However, I'm not finding this terribly useful, mostly due to it starting
comparison at the beginning of each menu line.

E.g., I'd like to be able to find "emulations" in the top-level menu here:
Executable file formats / Emulations --->

or I'd like to be able to find "crc" menu items in any sub-menu.
Maybe I should just stick to config symbol searches. I don't think it's all
that likely that people will know how each menu line text begins.


As for the search UI, I'd rather that it be presented like the symbol search,
in a box, instead of just a single line at the top of the screen.


> Signed-off-by: Nir Tzachar <[email protected]>
> ---
> scripts/kconfig/nconf.c | 350 +++++++++++++++++++++++++------------------
> scripts/kconfig/nconf.gui.c | 20 ++--
> scripts/kconfig/nconf.h | 3 +-
> 3 files changed, 213 insertions(+), 160 deletions(-)


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

2010-08-05 18:14:01

by Nir Tzachar

[permalink] [raw]
Subject: Re: [PATCH] nconfig: add search support

On Thu, Aug 5, 2010 at 8:04 PM, Randy Dunlap <[email protected]> wrote:
> On Tue, ?3 Aug 2010 15:28:12 +0300 [email protected] wrote:
>
>> From: Nir Tzachar <[email protected]>
>>
>> Remove the old hotkeys feature, and replace by a regular string search.
>> Behaviour of search is as advised by Sam.
>>
>> From nconfig help:
>>
>> Searching: pressing '/' triggers search mode. nconfig performs a
>> ? ? ? ? ? ?regular string compare, case insensitive, starting at
>
> I would say: simple string compare
> "regular" has no meaning (at least for me) there.

Regular means strcasecmp...

>> ? ? ? ? ? ?the beginning of each menu line.\n"
>> ? ? ? ? ? ?Pressing the up/down keys highlights the previous/next
>> ? ? ? ? ? ?matching item. Backspace removes one character from the
>> ? ? ? ? ? ?match string. Pressing either '/' again or ESC exits\
>> ? ? ? ? ? ?search mode. All other keys behave normally.
>>
>> Miscellaneous other changes (including Rundy's Justin's input).
>
> Yes, the bottom menu and the typos are fixed. ?Thanks.
>
> However, I'm not finding this terribly useful, mostly due to it starting
> comparison at the beginning of each menu line.
>
> E.g., I'd like to be able to find "emulations" in the top-level menu here:
> ? ? ? ?Executable file formats / Emulations ?--->
> or I'd like to be able to find "crc" menu items in any sub-menu.
> Maybe I should just stick to config symbol searches. ?I don't think it's all
> that likely that people will know how each menu line text begins.
>
>

We can replace strcasecmp with strcasestr. I agree it would be more useful.

> As for the search UI, I'd rather that it be presented like the symbol search,
> in a box, instead of just a single line at the top of the screen.

But then it is not interactive. I was aiming for something similar to
vim's search, where the search is matched as you type and the only
free terminal real-estate to display the match string was at the top
of the screen. I think such a minimal design is better than a
cumbersome text box which displays the search results afterwards (as
is symbol search), as the search is only intended for the currently
displayed menu and the user would usually just want to save the extra
typing of navigating to a specific menu item.

Cheers.

>> Signed-off-by: Nir Tzachar <[email protected]>
>> ---
>> ?scripts/kconfig/nconf.c ? ? | ?350 +++++++++++++++++++++++++------------------
>> ?scripts/kconfig/nconf.gui.c | ? 20 ++--
>> ?scripts/kconfig/nconf.h ? ? | ? ?3 +-
>> ?3 files changed, 213 insertions(+), 160 deletions(-)
>
>
> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>

2010-08-05 18:19:33

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] nconfig: add search support

On 08/05/10 11:13, Nir Tzachar wrote:
> On Thu, Aug 5, 2010 at 8:04 PM, Randy Dunlap <[email protected]> wrote:
>> On Tue, 3 Aug 2010 15:28:12 +0300 [email protected] wrote:
>>
>>> From: Nir Tzachar <[email protected]>
>>>
>>> Remove the old hotkeys feature, and replace by a regular string search.
>>> Behaviour of search is as advised by Sam.
>>>
>>> From nconfig help:
>>>
>>> Searching: pressing '/' triggers search mode. nconfig performs a
>>> regular string compare, case insensitive, starting at
>>
>> I would say: simple string compare
>> "regular" has no meaning (at least for me) there.
>
> Regular means strcasecmp...

That's not meaningful to a non-developer user.
I'm a developer and it still has little meaning to me in this context.


>>> the beginning of each menu line.\n"
>>> Pressing the up/down keys highlights the previous/next
>>> matching item. Backspace removes one character from the
>>> match string. Pressing either '/' again or ESC exits\
>>> search mode. All other keys behave normally.
>>>
>>> Miscellaneous other changes (including Rundy's Justin's input).
>>
>> Yes, the bottom menu and the typos are fixed. Thanks.
>>
>> However, I'm not finding this terribly useful, mostly due to it starting
>> comparison at the beginning of each menu line.
>>
>> E.g., I'd like to be able to find "emulations" in the top-level menu here:
>> Executable file formats / Emulations --->
>> or I'd like to be able to find "crc" menu items in any sub-menu.
>> Maybe I should just stick to config symbol searches. I don't think it's all
>> that likely that people will know how each menu line text begins.
>>
>>
>
> We can replace strcasecmp with strcasestr. I agree it would be more useful.

Yes.

>> As for the search UI, I'd rather that it be presented like the symbol search,
>> in a box, instead of just a single line at the top of the screen.
>
> But then it is not interactive. I was aiming for something similar to
> vim's search, where the search is matched as you type and the only
> free terminal real-estate to display the match string was at the top
> of the screen. I think such a minimal design is better than a
> cumbersome text box which displays the search results afterwards (as
> is symbol search), as the search is only intended for the currently
> displayed menu and the user would usually just want to save the extra
> typing of navigating to a specific menu item.

OK, I expect that I'll just mostly use symbol search instead of this one then.

The kicker for me is not how/where the dialog is -- it's this comment:

"as the search is only intended for the currently displayed menu"

I think that's much too limiting.


> Cheers.
>
>>> Signed-off-by: Nir Tzachar <[email protected]>
>>> ---
>>> scripts/kconfig/nconf.c | 350 +++++++++++++++++++++++++------------------
>>> scripts/kconfig/nconf.gui.c | 20 ++--
>>> scripts/kconfig/nconf.h | 3 +-
>>> 3 files changed, 213 insertions(+), 160 deletions(-)


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

2010-08-06 12:41:21

by Nir Tzachar

[permalink] [raw]
Subject: Re: [PATCH] nconfig: add search support

On Thu, Aug 5, 2010 at 9:19 PM, Randy Dunlap <[email protected]> wrote:
> On 08/05/10 11:13, Nir Tzachar wrote:
>> On Thu, Aug 5, 2010 at 8:04 PM, Randy Dunlap <[email protected]> wrote:
>>> On Tue, ?3 Aug 2010 15:28:12 +0300 [email protected] wrote:
>>>
>>>> From: Nir Tzachar <[email protected]>
>>>>
>>>> Remove the old hotkeys feature, and replace by a regular string search.
>>>> Behaviour of search is as advised by Sam.
>>>>
>>>> From nconfig help:
>>>>
>>>> Searching: pressing '/' triggers search mode. nconfig performs a
>>>> ? ? ? ? ? ?regular string compare, case insensitive, starting at
>>>
>>> I would say: simple string compare
>>> "regular" has no meaning (at least for me) there.
>>
>> Regular means strcasecmp...
>
> That's not meaningful to a non-developer user.
> I'm a developer and it still has little meaning to me in this context.
>
>
>>>> ? ? ? ? ? ?the beginning of each menu line.\n"
>>>> ? ? ? ? ? ?Pressing the up/down keys highlights the previous/next
>>>> ? ? ? ? ? ?matching item. Backspace removes one character from the
>>>> ? ? ? ? ? ?match string. Pressing either '/' again or ESC exits\
>>>> ? ? ? ? ? ?search mode. All other keys behave normally.
>>>>
>>>> Miscellaneous other changes (including Rundy's Justin's input).
>>>
>>> Yes, the bottom menu and the typos are fixed. ?Thanks.
>>>
>>> However, I'm not finding this terribly useful, mostly due to it starting
>>> comparison at the beginning of each menu line.
>>>
>>> E.g., I'd like to be able to find "emulations" in the top-level menu here:
>>> ? ? ? ?Executable file formats / Emulations ?--->
>>> or I'd like to be able to find "crc" menu items in any sub-menu.
>>> Maybe I should just stick to config symbol searches. ?I don't think it's all
>>> that likely that people will know how each menu line text begins.
>>>
>>>
>>
>> We can replace strcasecmp with strcasestr. I agree it would be more useful.
>
> Yes.
>
>>> As for the search UI, I'd rather that it be presented like the symbol search,
>>> in a box, instead of just a single line at the top of the screen.
>>
>> But then it is not interactive. I was aiming for something similar to
>> vim's search, where the search is matched as you type and the only
>> free terminal real-estate to display the match string was at the top
>> of the screen. I think such a minimal design is better than a
>> cumbersome text box which displays the search results afterwards (as
>> is symbol search), as the search is only intended for the currently
>> displayed menu and the user would usually just want to save the extra
>> typing of navigating to a specific menu item.
>
> OK, I expect that I'll just mostly use symbol search instead of this one then.
>
> The kicker for me is not how/where the dialog is -- it's this comment:
>
> ?"as the search is only intended for the currently displayed menu"
>
> I think that's much too limiting.

A global search with an instant goto upon match cannot easily be
incorporated into nconfig's (or menuconfig's) design, as other menus
than the current one are not present.

>
>> Cheers.
>>
>>>> Signed-off-by: Nir Tzachar <[email protected]>
>>>> ---
>>>> ?scripts/kconfig/nconf.c ? ? | ?350 +++++++++++++++++++++++++------------------
>>>> ?scripts/kconfig/nconf.gui.c | ? 20 ++--
>>>> ?scripts/kconfig/nconf.h ? ? | ? ?3 +-
>>>> ?3 files changed, 213 insertions(+), 160 deletions(-)
>
>
> --
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>

2010-08-06 13:54:34

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] nconfig: add search support

> >>
> >> Searching: pressing '/' triggers search mode. nconfig performs a
> >> ? ? ? ? ? ?regular string compare, case insensitive, starting at
> >
> > I would say: simple string compare
> > "regular" has no meaning (at least for me) there.

pressing '/' triggers interactive search mode. nconfig search for the
string in the menu prompts (no regex support).

[Just a suggestion for a bt different wording]

> > Maybe I should just stick to config symbol searches. ?I don't think it's all
> > that likely that people will know how each menu line text begins.
> >
> >
>
> We can replace strcasecmp with strcasestr. I agree it would be more useful.

This is better.

>
> > As for the search UI, I'd rather that it be presented like the symbol search,
> > in a box, instead of just a single line at the top of the screen.
>
> But then it is not interactive. I was aiming for something similar to
> vim's search, where the search is matched as you type and the only
> free terminal real-estate to display the match string was at the top
> of the screen. I think such a minimal design is better than a
> cumbersome text box which displays the search results afterwards (as
> is symbol search), as the search is only intended for the currently
> displayed menu and the user would usually just want to save the extra
> typing of navigating to a specific menu item.

It was introduced to replace the "hotkey" support, and as such is useful.
If we want to search for content of all prompts then we should extend
the symbols search to do so.
Maybe we should just let it search for both symbols _and_ propmts.

If one search for HOTPLUG_CPU there is no hits in any propmts anyway.
And if one search for "Pentium" there is no config symbol hits.

Sam

2010-08-06 14:09:28

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] nconfig: add search support

>
> or I'd like to be able to find "crc" menu items in any sub-menu.
> Maybe I should just stick to config symbol searches. I don't think it's all
> that likely that people will know how each menu line text begins.

Following patch extends symbols search to search _both_ for CONFIG_
symbols AND prompts.

I think this could be a usefull extension.
You are navigation much more around than me in the
Kconfig files / editors. What do you think?

As this is an extension in the core part it will take effect
for all the editors (which is good).
[gconf does not use sym_re_search() - I assume this feature is
missing in that editor].

I will cook up a proper patch only if I get positive feedback.

Sam

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index e95718f..8cda9c0 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -842,6 +842,7 @@ struct symbol *sym_find(const char *name)

struct symbol **sym_re_search(const char *pattern)
{
+ struct property *prop;
struct symbol *sym, **sym_arr = NULL;
int i, cnt, size;
regex_t re;
@@ -854,9 +855,15 @@ struct symbol **sym_re_search(const char *pattern)
return NULL;

for_all_symbols(i, sym) {
+ bool found = false;
if (sym->flags & SYMBOL_CONST || !sym->name)
continue;
- if (regexec(&re, sym->name, 0, NULL, 0))
+ if (!regexec(&re, sym->name, 0, NULL, 0))
+ found = true;
+ for_all_prompts(sym, prop)
+ if (!regexec(&re, prop->text, 0, NULL, 0))
+ found = true;
+ if (!found)
continue;
if (cnt + 1 >= size) {
void *tmp = sym_arr;

2010-08-06 19:22:17

by Nir Tzachar

[permalink] [raw]
Subject: Re: [PATCH] nconfig: add search support

On Fri, Aug 6, 2010 at 4:54 PM, Sam Ravnborg <[email protected]> wrote:
>> >>
>> >> Searching: pressing '/' triggers search mode. nconfig performs a
>> >> ? ? ? ? ? ?regular string compare, case insensitive, starting at
>> >
>> > I would say: simple string compare
>> > "regular" has no meaning (at least for me) there.
>
> pressing '/' triggers interactive search mode. nconfig search for the
> string in the menu prompts (no regex support).
>
> [Just a suggestion for a bt different wording]
>
>> > Maybe I should just stick to config symbol searches. ?I don't think it's all
>> > that likely that people will know how each menu line text begins.
>> >
>> >
>>
>> We can replace strcasecmp with strcasestr. I agree it would be more useful.
>
> This is better.
>
>>
>> > As for the search UI, I'd rather that it be presented like the symbol search,
>> > in a box, instead of just a single line at the top of the screen.
>>
>> But then it is not interactive. I was aiming for something similar to
>> vim's search, where the search is matched as you type and the only
>> free terminal real-estate to display the match string was at the top
>> of the screen. I think such a minimal design is better than a
>> cumbersome text box which displays the search results afterwards (as
>> is symbol search), as the search is only intended for the currently
>> displayed menu and the user would usually just want to save the extra
>> typing of navigating to a specific menu item.
>
> It was introduced to replace the "hotkey" support, and as such is useful.
> If we want to search for content of all prompts then we should extend
> the symbols search to do so.
> Maybe we should just let it search for both symbols _and_ propmts.

There is a different alternative which slipped my mind, which can be
useful for Randy's use case: just use the single menu mode of nconfig
and then the search feature will give you the behavior you desire
(i.e., jumping to the globally matching prompt).

> If one search for HOTPLUG_CPU there is no hits in any propmts anyway.
> And if one search for "Pentium" there is no config symbol hits.

If such a search cannot return the menu context of the matching
symbol, there is no way for nconfig to jump to the matching menu. But
a different approach may be to add the symbol search to nconfig's
search, such that instead of just matching the prompt, nconfig will
also match the symbol.

Cheers,
Nir.

> ? ? ? ?Sam
>