Xapian devel - Mar 2013 - patch-Add standard ExpandDecider subclass to restrict to terms with a particular prefix

Hi, guys, I wrote a patch for  ticket
#467<http://trac.xapian.org/ticket/467>(Add
standard ExpandDecider subclass to restrict to terms with a particular
prefix).
A new ExpandDecider class is added in expanddecider.cc/h, which delete all
unprefix terms

I am a newbiee to open source contribution, please let me know if this
patch is work.

Patch is here(generate by svn diff):
https://github.com/white127/xapian-patch/blob/master/Expand.patch

It would be nice for all your answers.

Regards
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.xapian.org/pipermail/xapian-devel/attachments/20130313/9aa0ec3a/attachment.htm>

On Wed, Mar 13, 2013 at 10:06:08AM +0800, jiangwen jiang
wrote:
> Hi, guys, I wrote a patch for  ticket
> #467<http://trac.xapian.org/ticket/467>(Add
> standard ExpandDecider subclass to restrict to terms with a particular
> prefix).
> A new ExpandDecider class is added in expanddecider.cc/h, which delete all
> unprefix terms
Thanks for your patch.
> I am a newbiee to open source contribution, please let me know if this
> patch is work.
> 
> Patch is here(generate by svn diff):
You probably want to use git rather than svn.  We're intending to change
to using git as the master system and stop using svn at all in the near
future (we just need to finished updating a few maintainer scripts).
> https://github.com/white127/xapian-patch/blob/master/Expand.patch
It looks pretty good.

A few comments:

* You don't need to store the length of the string separately - the
  std::string::length() method in C++ just returns a stored length - it
  isn't like strlen() in C - so calling it each time isn't a problem.

* We have a utility function called "startswith()" in Xapian (see
  common/stringutils.h) which provides a particularly efficient
  implementation of exactly the operation you need in operator().

* It really needs a feature test adding to the testsuite to ensure it
  works, and so that we know right away if somebody manages to break it
  with future changes.

Cheers,
    Olly