<div dir="ltr">Thanks for the feedback! <div>I have created a pull request with the necessary changes to the docs and with proper indentation. I have submitted the pull request to the master branch of the xapian project. Should I have posted it in some other branch? I wasn't sure about this.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 15, 2016 at 5:08 PM, James Aylett <span dir="ltr"><<a href="mailto:james-xapian@tartarus.org" target="_blank">james-xapian@tartarus.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi, Ayush — congratulations on getting stuck into this! As a matter of course, can you please keep replies copied to the mailing list so everyone can help and benefit from the discussion? [xapian-devel added back to cc.]<br>
<br>
Because you haven’t provided this as a patch or PR (more on that later), it’s harder for me to incorporate into my codebase and test, and I haven’t had time to do that yet. However it looks in principal good to me, with a couple of comments. (You should also try to ensure that you match our indentation style.)<br>
<br>
I think you’re making life more complex than you need to in creating the string version of the integer offset. Have a look at how the `$add{}` command does it (ll 1253–1260    <a href="https://github.com/xapian/xapian/blob/master/xapian-applications/omega/query.cc#L1253" rel="noreferrer" target="_blank">https://github.com/xapian/xapian/blob/master/xapian-applications/omega/query.cc#L1253</a>).<br>
<br>
You’ll also need to add documentation to `docs/omegascript.rst` to complete this project; just a short sentence describing what it does will be sufficient.<br>
<br>
To move this further forward, you also need to provide a patch in some form rather than a gist that isn’t formatting as a patch. The easiest way of doing this is probably to fork the Xapian project on github, push a commit to a branch there and then open a pull request. (Alternatively you can create a patch by using git locally, avoiding github. However since github is currently so popular among open source projects, it’s a good idea to learn how to use it.)<br>
<br>
Github has some documentation on forking and pull requests which may help (<a href="https://help.github.com/categories/collaborating-on-projects-using-pull-requests/" rel="noreferrer" target="_blank">https://help.github.com/categories/collaborating-on-projects-using-pull-requests/</a>), but if you have problems, please shout out and someone will try to help!<br>
<span class="HOEnZb"><font color="#888888"><br>
J<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> On 15 Feb 2016, at 10:56, Ayush Gupta <<a href="mailto:ayushgp10@gmail.com">ayushgp10@gmail.com</a>> wrote:<br>
><br>
> Hi James,<br>
> I have implemented the function here: <a href="https://gist.github.com/ayushgp/fd0b23a6d214e451edf1" rel="noreferrer" target="_blank">https://gist.github.com/ayushgp/fd0b23a6d214e451edf1</a><br>
> Please have a look.<br>
><br>
><br>
> On Mon, Feb 15, 2016 at 6:11 AM, James Aylett <<a href="mailto:james-xapian@tartarus.org">james-xapian@tartarus.org</a>> wrote:<br>
> On 14 Feb 2016, at 18:52, Ayush Gupta <<a href="mailto:ayushgp10@gmail.com">ayushgp10@gmail.com</a>> wrote:<br>
><br>
> > I tried implementing the bite sized project idea posted here: <a href="https://trac.xapian.org/wiki/ProjectIdeas#AddnewOmegaScriptcommandtodoasubstringsearch" rel="noreferrer" target="_blank">https://trac.xapian.org/wiki/ProjectIdeas#AddnewOmegaScriptcommandtodoasubstringsearch</a> but could not understand what needs to be returned when this command is called.<br>
><br>
> Hi, Ayush. It’s probably worth you creating a short plan as you figure out what’s needed here. Having a plan before you write code is always a good idea, and if you create it as a gist or similar, you can share it with other people on the list when asking questions. It’ll help others know where you’ve got to and what you’ve tried, and is good practice for when you get into larger projects (such as GSoC) which last weeks or months.<br>
><br>
> > This line is not so clear to me: expand to the offset of the first occurrence of fish from the start of the string if $query, or to if $query doesn't contain the substring fish.<br>
><br>
> Based on the description, here are some examples of what `$contains{fish,$query}` would expand to based on different values of `$query`:<br>
><br>
> $query          $contains{fish,$query}<br>
> --------------------------------------<br>
> hello           (empty string)<br>
> fish            0<br>
> hello fish      6<br>
><br>
> So you’d be able to do things like `$if{$contains{fish,$query},fish present,no fishes}`.<br>
><br>
> Hopefully that makes it more clear.<br>
><br>
> > Also in query.cc file in omega dir, when defining function descriptions, what does the evalargs and ensure arguments mean?<br>
><br>
> ensure means that the command requires either a query (‘Q’) or a query and a match (‘M’) to have been evaluated in the script up to that point. (See lines 1246–1248 of query.cc.)<br>
><br>
> evalargs (ll 1238–1244) controls the number of arguments to the command that are evaluated, ie that themselves can contain OmegaScript commands. (Some commands explicitly evaluate some of their arguments themselves, so this is more convenience where a number of the arguments need evaluating unconditionally — such as for `$def{}` where the macroname is evaluated, but the expansion of the macro is not, and `$if{}` where we only want to evaluate one of the result branches, not both.)<br>
><br>
> Ideally they’d both be explained in comments in the source code (around the structure definition and macro). Feel free to make a small patch and pull request to cover that!<br>
><br>
> (Note that it looks to me like there’s a bug in the evalargs handling, where minargs == N.)<br>
><br>
> J<br>
><br>
> --<br>
>  James Aylett, occasional trouble-maker<br>
>  <a href="http://xapian.org" rel="noreferrer" target="_blank">xapian.org</a><br>
><br>
><br>
<br>
--<br>
 James Aylett, occasional trouble-maker<br>
 <a href="http://xapian.org" rel="noreferrer" target="_blank">xapian.org</a><br>
<br>
</div></div></blockquote></div><br></div>