2007-09-14 14:33:25

by Shlomi Fish

[permalink] [raw]
Subject: [PATCH 2.6.23-rc4] qconf ("make xconfig") Search Dialog Enhancement (rev8)

Hi all!

[ I'm not subscribed to this list so please CC me on your replies. ]

This patch adds an option to use either substring match, regular expression
match, or keywords search in the "make xconfig" Edit->Find dialog.

It also allows searching on the "help" field of the menus as well as
the "name" of the symbol.

This change involved adding the sym_generic_search function to the LKC, and
implementing sym_re_search using it.

Regards,

Shlomi Fish

--- linux-2.6.23-rc4/scripts/kconfig/qconf.cc.orig 2007-09-01
21:15:39.017466409 +0300
+++ linux-2.6.23-rc4/scripts/kconfig/qconf.cc 2007-09-08 11:29:31.357869409
+0300
@@ -1199,6 +1199,23 @@
layout2->addWidget(searchButton);
layout1->addLayout(layout2);

+ // Initialize the "Search Type" button group.
+ searchType = new QVButtonGroup("Search Type", this, "searchType");
+
+ substringSearch = new QRadioButton(
+ "Substring Match", searchType, "Substring Match"
+ );
+
+ keywordsSearch = new QRadioButton("Keywords", searchType, "Keywords");
+
+ regexSearch = new QRadioButton(
+ "Regular Expression", searchType, "Regular Expression"
+ );
+
+ substringSearch->setChecked(TRUE);
+ layout1->addWidget(searchType);
+
+ // Initialize the ConfigView and ConfigInfoView
split = new QSplitter(this);
split->setOrientation(QSplitter::Vertical);
list = new ConfigView(split, name);
@@ -1245,6 +1262,20 @@
}
}

+SEARCH_TYPE ConfigSearchWindow::getSearchType()
+{
+ return substringSearch->isChecked() ? SUBSTRING :
+ regexSearch->isChecked() ? REGEX :
+ KEYWORDS;
+}
+
+
+static int search_filter(struct symbol *sym, void *context)
+{
+ FindQuery *query = (FindQuery *)context;
+ return query->sym_matches(sym);
+}
+
void ConfigSearchWindow::search(void)
{
struct symbol **p;
@@ -1255,7 +1286,9 @@
list->list->clear();
info->clear();

- result = sym_re_search(editField->text().latin1());
+ FindQuery query(editField->text(), getSearchType());
+
+ result = sym_generic_search(search_filter, (void *)&query);
if (!result)
return;
for (p = result; *p; p++) {
@@ -1265,6 +1298,114 @@
}
}

+FindQuery::FindQuery(QString q, SEARCH_TYPE t, bool a_case) :
+ query(q), search_type(t), case_sensitive(a_case)
+{
+ compiled_regex = NULL;
+ if (search_type == REGEX)
+ {
+ compiled_regex = new QRegExp(query, case_sensitive);
+ }
+ else if (search_type == KEYWORDS)
+ {
+ compile_keywords();
+ }
+}
+
+void FindQuery::compile_keywords()
+{
+ QRegExp split_words("\\s+");
+ QStringList bare_words = QStringList::split(split_words, query);
+
+ QStringList::iterator it;
+ for ( it = bare_words.begin(); it != bare_words.end(); ++it )
+ {
+ QString escaped = QRegExp::escape(*it);
+
+ keywords.append(QRegExp(QString("\\b") + escaped + "\\b"));
+ }
+}
+
+FindQuery::~FindQuery()
+{
+ if (compiled_regex)
+ {
+ delete compiled_regex;
+ compiled_regex = NULL;
+ }
+}
+
+bool FindQuery::string_matches_regex(QRegExp & re, const char *string)
+{
+ QString qs(string);
+
+ return (re.search(qs) >= 0);
+}
+
+bool FindQuery::string_matches(const char *string)
+{
+ QString qs(string);
+
+ if (search_type == SUBSTRING)
+ {
+ return qs.contains(query, case_sensitive);
+ }
+ else if (search_type == REGEX)
+ {
+ return string_matches_regex(*compiled_regex, string);
+ }
+ else
+ {
+ return false;
+ }
+}
+
+bool FindQuery::sym_matches(struct symbol *sym)
+{
+ if (!sym)
+ {
+ return false;
+ }
+
+ if (search_type == KEYWORDS)
+ {
+ return sym_matches_keywords(sym);
+ }
+ else
+ {
+ return sym_matches_atom(sym);
+ }
+
+}
+
+bool FindQuery::sym_matches_atom(struct symbol *sym)
+{
+ return (string_matches(sym->name) ||
+ (sym->prop &&
+ sym->prop->menu &&
+ string_matches(sym->prop->menu->help)
+ ));
+}
+
+bool FindQuery::sym_matches_keywords(struct symbol *sym)
+{
+ QValueList<QRegExp>::iterator it;
+
+ for ( it = keywords.begin(); it != keywords.end(); ++it )
+ {
+ if (!(string_matches_regex(*it, sym->name)
+ ||
+ (sym->prop &&
+ sym->prop->menu &&
+ string_matches_regex(*it, sym->prop->menu->help))))
+ {
+ return false;
+ }
+ }
+
+ return true;
+}
+
/*
* Construct the complete config widget
*/
--- linux-2.6.23-rc4/scripts/kconfig/qconf.h.orig 2007-09-01
21:15:33.537030610 +0300
+++ linux-2.6.23-rc4/scripts/kconfig/qconf.h 2007-09-08 11:12:18.015699416
+0300
@@ -4,6 +4,8 @@
*/

#include <qlistview.h>
+#include <qradiobutton.h>
+#include <qvbuttongroup.h>
#if QT_VERSION >= 300
#include <qsettings.h>
#else
@@ -275,6 +277,7 @@
bool _showDebug;
};

+enum SEARCH_TYPE { SUBSTRING, KEYWORDS, REGEX };
class ConfigSearchWindow : public QDialog {
Q_OBJECT
typedef class QDialog Parent;
@@ -288,11 +291,15 @@
protected:
QLineEdit* editField;
QPushButton* searchButton;
+ QVButtonGroup *searchType;
+ QRadioButton *substringSearch, *keywordsSearch, *regexSearch;
QSplitter* split;
ConfigView* list;
ConfigInfoView* info;

struct symbol **result;
+
+ SEARCH_TYPE getSearchType();
};

class ConfigMainWindow : public QMainWindow {
@@ -332,3 +339,25 @@
QSplitter* split1;
QSplitter* split2;
};
+
+class FindQuery
+{
+ public:
+ bool case_sensitive;
+ SEARCH_TYPE search_type;
+ QString query;
+ QRegExp * compiled_regex;
+ QValueList<QRegExp> keywords;
+
+ public:
+ FindQuery() : query("") { case_sensitive = false; search_type = SUBSTRING; }
+ FindQuery(QString q, SEARCH_TYPE t, bool a_case = false);
+ ~FindQuery();
+ void compile_keywords();
+ bool string_matches(const char *);
+ bool string_matches_regex(QRegExp & re, const char *string);
+ bool sym_matches(struct symbol *sym);
+ bool sym_matches_keywords(struct symbol *sym);
+ bool sym_matches_atom(struct symbol *sym);
+};
+
--- linux-2.6.23-rc4/scripts/kconfig/lkc.h.orig 2007-09-01 19:56:36.528350007
+0300
+++ linux-2.6.23-rc4/scripts/kconfig/lkc.h 2007-09-01 19:57:57.758809341 +0300
@@ -111,6 +111,9 @@
struct property *prop_alloc(enum prop_type type, struct symbol *sym);
struct symbol *prop_get_symbol(struct property *prop);

+typedef int (*sym_search_filter_t)(struct symbol *sym, void *context);
+struct symbol **sym_generic_search(sym_search_filter_t filter, void
*context);
+
static inline tristate sym_get_tristate_value(struct symbol *sym)
{
return sym->curr.tri;
--- linux-2.6.23-rc4/scripts/kconfig/symbol.c.orig 2007-09-01
21:24:07.885930947 +0300
+++ linux-2.6.23-rc4/scripts/kconfig/symbol.c 2007-09-01 20:41:58.420791258
+0300
@@ -717,23 +717,39 @@
return symbol;
}

+static int re_search_filter(struct symbol *sym, void *re_void)
+{
+ return (regexec((regex_t*)re_void, sym->name, 0, NULL, 0) == 0);
+}
+
struct symbol **sym_re_search(const char *pattern)
{
- struct symbol *sym, **sym_arr = NULL;
- int i, cnt, size;
regex_t re;
+ struct symbol **results;

- cnt = size = 0;
- /* Skip if empty */
if (strlen(pattern) == 0)
return NULL;
if (regcomp(&re, pattern, REG_EXTENDED|REG_NOSUB|REG_ICASE))
return NULL;

+ results = sym_generic_search(re_search_filter, (void *)&re);
+
+ regfree(&re);
+
+ return results;
+}
+
+struct symbol **sym_generic_search(sym_search_filter_t filter, void *context)
+{
+ struct symbol *sym, **sym_arr = NULL;
+ int i, cnt, size;
+
+ cnt = size = 0;
+
for_all_symbols(i, sym) {
if (sym->flags & SYMBOL_CONST || !sym->name)
continue;
- if (regexec(&re, sym->name, 0, NULL, 0))
+ if (!filter(sym, context))
continue;
if (cnt + 1 >= size) {
void *tmp = sym_arr;
@@ -748,7 +764,6 @@
}
if (sym_arr)
sym_arr[cnt] = NULL;
- regfree(&re);

return sym_arr;
}


---------------------------------------------------------------------
Shlomi Fish [email protected]
Homepage: http://www.shlomifish.org/

If it's not in my E-mail it doesn't happen. And if my E-mail is saying
one thing, and everything else says something else - E-mail will conquer.
-- An Israeli Linuxer
--

---------------------------------------------------------------------
Shlomi Fish [email protected]
Homepage: http://www.shlomifish.org/

If it's not in my E-mail it doesn't happen. And if my E-mail is saying
one thing, and everything else says something else - E-mail will conquer.
-- An Israeli Linuxer


2007-09-14 21:10:25

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 2.6.23-rc4] qconf ("make xconfig") Search Dialog Enhancement (rev8)

Hi Shlomi Fish.

> [ I'm not subscribed to this list so please CC me on your replies. ]
We always do so at lkml..

> This patch adds an option to use either substring match, regular expression
> match, or keywords search in the "make xconfig" Edit->Find dialog.
>
> It also allows searching on the "help" field of the menus as well as
> the "name" of the symbol.
>
> This change involved adding the sym_generic_search function to the LKC, and
> implementing sym_re_search using it.
>
Thanks for being persistent on this.
Unless I hear otherwise from Roman I plan to apply your patch during the weekend.

Sam

2007-09-16 09:23:48

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 2.6.23-rc4] qconf ("make xconfig") Search Dialog Enhancement (rev8)

On Fri, Sep 14, 2007 at 04:45:03PM +0300, Shlomi Fish wrote:
> Hi all!
>
> [ I'm not subscribed to this list so please CC me on your replies. ]
>
> This patch adds an option to use either substring match, regular expression
> match, or keywords search in the "make xconfig" Edit->Find dialog.
>
> It also allows searching on the "help" field of the menus as well as
> the "name" of the symbol.
>
> This change involved adding the sym_generic_search function to the LKC, and
> implementing sym_re_search using it.

I fixed up the line breake in the patch and applied it.
Please add a signed-off-by next time.

Thanks,
Sam

2007-09-16 16:43:04

by Roman Zippel

[permalink] [raw]
Subject: Re: [PATCH 2.6.23-rc4] qconf ("make xconfig") Search Dialog Enhancement (rev8)

Hi,

On Thu, 13 Sep 2007, Shlomi Fish wrote:

> This patch adds an option to use either substring match, regular expression
> match, or keywords search in the "make xconfig" Edit->Find dialog.
>
> It also allows searching on the "help" field of the menus as well as
> the "name" of the symbol.
>
> This change involved adding the sym_generic_search function to the LKC, and
> implementing sym_re_search using it.

I like the direction, but first of all please fix the coding style.
I would also prefer to move more of the search functionality into the
generic code, so it can be used by other front ends as well, otherwise a
lot of this had to be duplicated.
I think a filter function makes it maybe a bit to flexible, if a front
end wants to do some weird filtering, it can still access the symbol
data base directly. So what I have in mind is something like this:

struct symbol **sym_generic_search(const char *pattern, unsigned int flags);

This means the back end provides a basic search facility for the most
common search operations. The flags would specify what to search (e.g.
symbol name, help text, prompts) and how to do it.


> @@ -1199,6 +1199,23 @@
> layout2->addWidget(searchButton);
> layout1->addLayout(layout2);
>
> + // Initialize the "Search Type" button group.
> + searchType = new QVButtonGroup("Search Type", this, "searchType");
> +
> + substringSearch = new QRadioButton(
> + "Substring Match", searchType, "Substring Match"
> + );
> +
> + keywordsSearch = new QRadioButton("Keywords", searchType, "Keywords");
> +
> + regexSearch = new QRadioButton(
> + "Regular Expression", searchType, "Regular Expression"
> + );
> +
> + substringSearch->setChecked(TRUE);
> + layout1->addWidget(searchType);
> +
> + // Initialize the ConfigView and ConfigInfoView
> split = new QSplitter(this);
> split->setOrientation(QSplitter::Vertical);
> list = new ConfigView(split, name);
> @@ -1245,6 +1262,20 @@
> }
> }
>
> +SEARCH_TYPE ConfigSearchWindow::getSearchType()
> +{
> + return substringSearch->isChecked() ? SUBSTRING :
> + regexSearch->isChecked() ? REGEX :
> + KEYWORDS;
> +}
>

If you use QButtonGroup::insert() you can assign id's to the indivual
buttons and then use selectedId to make this part simpler.

bye, Roman

2007-09-20 11:51:20

by Shlomi Fish

[permalink] [raw]
Subject: Re: [PATCH 2.6.23-rc4] qconf ("make xconfig") Search Dialog Enhancement (rev8)

Hi!

Sorry for the late response.

On Sunday 16 September 2007, Roman Zippel wrote:
> Hi,
>
> On Thu, 13 Sep 2007, Shlomi Fish wrote:
> > This patch adds an option to use either substring match, regular
> > expression match, or keywords search in the "make xconfig" Edit->Find
> > dialog.
> >
> > It also allows searching on the "help" field of the menus as well as
> > the "name" of the symbol.
> >
> > This change involved adding the sym_generic_search function to the LKC,
> > and implementing sym_re_search using it.
>
> I like the direction, but first of all please fix the coding style.

Which specific problems do you see with the coding style of the patch? Can you
comment on it?

> I would also prefer to move more of the search functionality into the
> generic code, so it can be used by other front ends as well, otherwise a
> lot of this had to be duplicated.

That would be a good idea, but I cannot use Qt there, which makes my job
harder.

> I think a filter function makes it maybe a bit to flexible, if a front
> end wants to do some weird filtering, it can still access the symbol
> data base directly.

A filter function would still be convenient in this context, as the symbol
data base API may change, and the filter function has a little logic in it.

> So what I have in mind is something like this:
>
> struct symbol **sym_generic_search(const char *pattern, unsigned int
> flags);
>
> This means the back end provides a basic search facility for the most
> common search operations. The flags would specify what to search (e.g.
> symbol name, help text, prompts) and how to do it.

I suggest we don't call it sym_generic_search, as generic implies it is a
generic filter. We can call it "sym_string_search" or whatever. Then, I
suggest we have separate arguments for every parameter (i.e: search type,
case sensitivity, what to search, etc.).



>
> > @@ -1199,6 +1199,23 @@
> > layout2->addWidget(searchButton);
> > layout1->addLayout(layout2);
> >
> > + // Initialize the "Search Type" button group.
> > + searchType = new QVButtonGroup("Search Type", this, "searchType");
> > +
> > + substringSearch = new QRadioButton(
> > + "Substring Match", searchType, "Substring Match"
> > + );
> > +
> > + keywordsSearch = new QRadioButton("Keywords", searchType, "Keywords");
> > +
> > + regexSearch = new QRadioButton(
> > + "Regular Expression", searchType, "Regular Expression"
> > + );
> > +
> > + substringSearch->setChecked(TRUE);
> > + layout1->addWidget(searchType);
> > +
> > + // Initialize the ConfigView and ConfigInfoView
> > split = new QSplitter(this);
> > split->setOrientation(QSplitter::Vertical);
> > list = new ConfigView(split, name);
> > @@ -1245,6 +1262,20 @@
> > }
> > }
> >
> > +SEARCH_TYPE ConfigSearchWindow::getSearchType()
> > +{
> > + return substringSearch->isChecked() ? SUBSTRING :
> > + regexSearch->isChecked() ? REGEX :
> > + KEYWORDS;
> > +}
>
> If you use QButtonGroup::insert() you can assign id's to the indivual
> buttons and then use selectedId to make this part simpler.
>

Ah, OK.

Regards,

Shlomi Fish

---------------------------------------------------------------------
Shlomi Fish [email protected]
Homepage: http://www.shlomifish.org/

If it's not in my E-mail it doesn't happen. And if my E-mail is saying
one thing, and everything else says something else - E-mail will conquer.
-- An Israeli Linuxer

2007-09-20 12:07:22

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 2.6.23-rc4] qconf ("make xconfig") Search Dialog Enhancement (rev8)

Hi Shlomi Fish.

When you address the comments of Roman just sent a replacemnt
patch. I will drop the one I have in kbuild.git then.

Sam

2007-10-05 02:08:58

by Roman Zippel

[permalink] [raw]
Subject: Re: [PATCH 2.6.23-rc4] qconf ("make xconfig") Search Dialog Enhancement (rev8)

Hi,

On Thu, 20 Sep 2007, Shlomi Fish wrote:

> Which specific problems do you see with the coding style of the patch? Can you
> comment on it?

Mostly whitespace around any braces, please keep it close to the other
source.

> > I would also prefer to move more of the search functionality into the
> > generic code, so it can be used by other front ends as well, otherwise a
> > lot of this had to be duplicated.
>
> That would be a good idea, but I cannot use Qt there, which makes my job
> harder.

Where is the problem with implementing it in C? Just try to keep it a
simple at first.

> > I think a filter function makes it maybe a bit to flexible, if a front
> > end wants to do some weird filtering, it can still access the symbol
> > data base directly.
>
> A filter function would still be convenient in this context, as the symbol
> data base API may change, and the filter function has a little logic in it.

This API is not really fixed at the moment, so it's not really a problem.

> > So what I have in mind is something like this:
> >
> > struct symbol **sym_generic_search(const char *pattern, unsigned int
> > flags);
> >
> > This means the back end provides a basic search facility for the most
> > common search operations. The flags would specify what to search (e.g.
> > symbol name, help text, prompts) and how to do it.
>
> I suggest we don't call it sym_generic_search, as generic implies it is a
> generic filter. We can call it "sym_string_search" or whatever. Then, I
> suggest we have separate arguments for every parameter (i.e: search type,
> case sensitivity, what to search, etc.).

I don't care much about the name, but please keep it as a simple flag,
which is a lot easier to extend.

bye, Roman

2007-10-11 17:21:20

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH 2.6.23-rc4] qconf ("make xconfig") Search Dialog Enhancement (rev8)

On Mon, Oct 08, 2007 at 09:55:11PM +0200, Shlomi Fish wrote:
> Hi!
>
> On Friday 05 October 2007, Roman Zippel wrote:
> > Hi,
> >
> > On Thu, 20 Sep 2007, Shlomi Fish wrote:
> > > Which specific problems do you see with the coding style of the patch?
> > > Can you comment on it?
> >
> > Mostly whitespace around any braces, please keep it close to the other
> > source.
> >
>
> I see. I'll see what I can do.
I have dropped your previous patch for now awaiting the update.

Sam