[Xapian-devel] Re: [Xapian-commits] 9476: trunk/xapian-core/ trunk/xapian-core/include/xapian/ trunk/xapian-core/queryparser/ trunk/xapian-core/tests/

[Richard Boulton]

Olly Betts wrote:
> The API as it is after this change is the original one you committed
> (with one small semantic change).  You only changed it because I pointed
> out that the behaviour it changed might actually be being used.  But I
> had thought that setting the same field to a different prefix changed
> it, whereas actually it's ignored, so the whole premise for changing it
> was flawed.

Sure, that makes sense.  I may be over confident that the API should 
evolve in the direction I changed it, with a view to adding further 
parameters to add_prefix() as time goes on, but it's perfectly sensible 
to stick with the current API for now and not add new forms of 
add_prefix() until we have implementations which need them (and 
therefore, know that we've got the API workable).

> I tried to bring this up on IRC, but got no response, so I went ahead
> as it didn't seem controversial (it was your original API after all),
> and SVN HEAD is currently much too far from being in a releasable state,
> which I want to address.  Ideally we should be able to make a release at
> short notice if we need to.

Also agreed.  (I saw the comment on IRC, about 12 hours later, but 
didn't respond since I didn't realise it was more than an observation.)

> FWIW, it also ends at a close bracket to avoid swallowing them when used at
> the end of a bracketted subexpression.

Forgot about that.

> I think it's better not to try to predict the API we'll want here before
> we actually try to implement it.  We may get it wrong and force users to
> update their add_prefix() calls multiple times.  Deprecating a method in
> favour of another has a cost for everyone using it, so it's not
> something we should do too lightly.

Good point.

>> The main concern I have is to check that we're still heading in the same 
>> direction: making the query parser more configurable, and decoupling the 
>> status of a prefix as an inline or a filter prefix from the way in which 
>> term processing is done.
> 
> I'm not sure I'd use the word decouple, as there are essential
> differences between the two (for example, it doesn't make sense to
> have the empty field prefix being a filter, nor to have a phrase of
> filters).  But certainly this should be much more configurable.

Good.

> FWIW, I find it slightly more readable as currently used - the choice is
> essentially:
> 
>     foo->filter
> 
> versus:
> 
>     foo->type == TYPE_FILTER
> 
> It also avoids the need to Assert(foo->type == TYPE_INLINE); after
> dealing with the boolean filter case.
> 
> But the main reason I changed it was that I find "TYPE_INLINE" a rather
> confusing name, and couldn't think of a better one.  To me, TYPE_INLINE
> suggests a boolean filter inline in the query string (constrasted with
> a boolean filter outside the query (e.g. in an HTML SELECT).

Okay, fair enough.

As long as we're going in the same direction, I'm happy. :)

-- 
Richard