Re: Support for integrated tsearch configuration

Dave Page a écrit :
> On Fri, Aug 1, 2008 at 11:05 AM, Guillaume Lelarge
> <guillaume(at)lelarge(dot)info> wrote:
>> I worked three days on it. The fourth was more about testing it on different
>> platforms (GTK, Windows, Mac). Now, It's working. I don't attach the patch
>> because it's really big, but here is a URL to get it compressed:
>
> Cool :-). The usual list of random thoughts....
>
> - Should we call objects 'FTS xxxx'? All the 'Text Search xxxx' labels
> look a little long.
>

Fixed. I put FTS on all menu items.

> - There are some tokens to add to the ctlSQLBox list - at least
> GETTOKEN, LEXTYPES, HEADLINE, INIT, LEXIZE
>

I suppose this is for highlighting the Text Search keywords?

I have a few questions about this. There's a wxString that lists all
PL/pgsql specific keywords. Should I do the same with Text Search ones?

Why are the PL/pgsql specific keywords out of the pgadmin/db/keywords.c
and pgadmin/include/parser/keywords.h files? And why are the
EnterpriseDB specific keywords in these files and not in a wxString like
the PL/pgsql ones?

Here are the FTS keywords I need to add:
* gettoken, lextypes, headline (FTS Parser)
* init, lexize (FTS Template)

These FTS keywords are already available:
* parser (FTS Configuration)
* template (FTS Dictionary)
* start, end (FTS Parser)

In the new patch, I'm using another wxString to make it work like the
PL/pgsql keywords.

> - There is a little inconsistency in the RE-SQL formatting - for a
> template for example we have:
>
> CREATE TEXT SEARCH TEMPLATE fred (
> INIT = dsimple_init,
> LEXIZE = dsimple_lexize);
>
> and for a dictionary:
>
> CREATE TEXT SEARCH DICTIONARY fred (
> TEMPLATE = "simple"
> );
>
> Note the ); position.
>

You're right. It's fixed to follow the second one.

> - I got a crash when trying to create a config with no tokens.
>
> 0 pgAdmin3-Debug 0x00021b63
> wxArrayString::GetCount() const + 9 (arrstr.h:144)
> 1 pgAdmin3-Debug 0x000fed06
> dlgTextSearchConfiguration::GetSql() + 1634
> (dlgTextSearchConfiguration.cpp:346)
> 2 pgAdmin3-Debug 0x000cbf7f
> dlgProperty::OnOK(wxCommandEvent&) + 335 (dlgProperty.cpp:759)
> ...
>

Fixed. I did so many tests that I didn't do this obvious one :/

> - The Dictionaries textbox is oddly sized on the Tokens tab of the
> Configuration.
>

Some explanations on the Dictionaries textbox. I wanted to have a real
textbox, so that anyone can type what they want in it (moreover, in the
order they want to have the dictionaries) and so that, if they choose
something in the combobox, it won't replace the actual content of the
textbox.

So, I shrinked the combobox to the lowest I can, so that users can't see
the text of the combobox, and I added a textbox before the combo box.
I'm not sure this is the best way to do it. I'm a bit afraid of some UI
issues but I didn't see one at the moment. And size seems good to me.

Can you send me a screenshot of your issue? Thanks.

> - The dialogue boxes default to different sizes. They should all be
> consistently sized.
>

Fixed. Unfortunately, now, the default size is also the minimum size.

> - Don't forget to add new headers to precomp.h.
>

Done.

> I only gave the code a cusory glance - you've got lot's of pgAdmin
> experience now so I trust that it's all as clean as the bits I looked
> at :-)
>
> Overall, looks pretty good :-)
>

Thanks :)

New patch on:
http://developer.pgadmin.org/~guillaume/fts_20080810.patch.bz2

--
Guillaume.
http://www.postgresqlfr.org
http://dalibo.com