Handle second and third argument to Xapian::StringValueRangeProcessor new() in Perl bindings

[Adam Sjøgren]

Search::Xapian lacks support for the three argument version of new for Xapian::StringValueRangeProcessor.

The attached patch tries to add support, and updates the documentation. We haven't had time to add to the test suite. The patch can be included under GPLv2 or later.

[Olly Betts]

We ask for a dual licence on contributed patches, not just GPLv2+:

​http://trac.xapian.org/browser/trunk/xapian-core/HACKING#L1231

In this particular case, the XS changes would only be relevant for 1.2.x as trunk has SWIG-based Perl bindings (which should wrap these optional arguments already), but the documentation change and any testsuite changes would likely be useful for trunk too.

Looking at the patch, I think passing 2 arguments should be valid too, not just 1 or 3.

A simple feature test to make sure the Perl wrapper does now accept 3 (or 2) arguments and still accepts 1 would be good, and would also help to ensure this is compatible with trunk. I can probably write one if necessary, though that's likely to delay getting this applied - a corresponding testsuite change would be very welcome.

[Adam Sjøgren]

License: ah, sorry, I misread and thought either was enough.

Trunk: I couldn't find search-xapian in trunk, that is why the patch is against 1.2.

2 arguments: Ok, I'm not proficient enough in C++ to tell.

Thanks for the feedback.

[Olly Betts]

On trunk the search-xapian module has been removed, and there's a SWIG-generated set of bindings (also called Search::Xapian) in xapian-bindings/perl. The big benefit of using SWIG is that wrapping of new features happens automatically in many cases, so you shouldn't end up hitting missing wrappers like this one as of 1.4.0.

The SWIG-generated wrappers are mostly compatible with those in search-xapian, though there are a few differences. I've been keeping the testsuites aligned as far as possible, which helps to minimise the differences.

[Olly Betts]

Oh, and I tried to make it clearer that we're after a dual-licence in r16835.

[Adam Sjøgren]

Updated patch

[Adam Sjøgren]

I have uploaded a new patch.

It now handles the 2 argument case, and adds two tests to parser.t (testing 2 and 3 argument calls; the existing tests cover the 1 argument case, as far as I can tell).

The content of the patch is contributed to the Xapian project as dual-licensed under GPLv2 and later, and MIT/X.

[Olly Betts]

Thanks, looks good from a quick scan through. Thanks for the update.

Actually, since 1.1.2, StringValueRangeProcessor handles the prefix/suffix and DateValueRangeProcessor and NumberValueRangeProcessor are now subclasses of it (previously, all three were direct subclasses of ValueRangeProcessor, and only NumberValueRangeProcessor had the prefix and suffix support, which is what the "valueranges" document describes).

So I've updated that document to describe the prefix/suffix stuff once for all the classes instead, and committed that change to trunk as r16836.

Still to do: r16836 needs backporting to the 1.2 branch, the rest of the patch needs applying to the 1.2 branch, and the new tests need to be added to the SWIG-based perl bindings on trunk.

[Olly Betts]

Committed rest of patch to 1.2 branch as r16837.

[Olly Betts]

New tests added to SWIG-based perl bindings on trunk in r16842 (and they pass, which is good).

Doc improvement in xapian-core backported for 1.2.13 in r16843.