Hi, I'm Ayush an undergraduate Computer Science student from Thapar university, India. I was fiddling with xapian since the morning and trying to understand the code and internals of Xapian. I tried implementing the bite sized project idea posted here: https://trac.xapian.org/wiki/ProjectIdeas#AddnewOmegaScriptcommandtodoasubstringsearch but could not understand what needs to be returned when this command is called. 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. Also in query.cc file in omega dir, when defining function descriptions, what does the evalargs and ensure arguments mean? Thanks! Ayush -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.xapian.org/pipermail/xapian-devel/attachments/20160215/81749947/attachment.html>
On 14 Feb 2016, at 18:52, Ayush Gupta <ayushgp10 at gmail.com> wrote:> I tried implementing the bite sized project idea posted here: https://trac.xapian.org/wiki/ProjectIdeas#AddnewOmegaScriptcommandtodoasubstringsearch but could not understand what needs to be returned when this command is called.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.> 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.Based on the description, here are some examples of what `$contains{fish,$query}` would expand to based on different values of `$query`: $query $contains{fish,$query} -------------------------------------- hello (empty string) fish 0 hello fish 6 So you?d be able to do things like `$if{$contains{fish,$query},fish present,no fishes}`. Hopefully that makes it more clear.> Also in query.cc file in omega dir, when defining function descriptions, what does the evalargs and ensure arguments mean?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.) 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.) 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! (Note that it looks to me like there?s a bug in the evalargs handling, where minargs == N.) J -- James Aylett, occasional trouble-maker xapian.org
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.]
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.)
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
https://github.com/xapian/xapian/blob/master/xapian-applications/omega/query.cc#L1253).
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.
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.)
Github has some documentation on forking and pull requests which may help
(https://help.github.com/categories/collaborating-on-projects-using-pull-requests/),
but if you have problems, please shout out and someone will try to help!
J
> On 15 Feb 2016, at 10:56, Ayush Gupta <ayushgp10 at gmail.com> wrote:
>
> Hi James,
> I have implemented the function here:
https://gist.github.com/ayushgp/fd0b23a6d214e451edf1
> Please have a look.
>
>
> On Mon, Feb 15, 2016 at 6:11 AM, James Aylett <james-xapian at
tartarus.org> wrote:
> On 14 Feb 2016, at 18:52, Ayush Gupta <ayushgp10 at gmail.com> wrote:
>
> > I tried implementing the bite sized project idea posted here:
https://trac.xapian.org/wiki/ProjectIdeas#AddnewOmegaScriptcommandtodoasubstringsearch
but could not understand what needs to be returned when this command is called.
>
> 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.
>
> > 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.
>
> Based on the description, here are some examples of what
`$contains{fish,$query}` would expand to based on different values of `$query`:
>
> $query $contains{fish,$query}
> --------------------------------------
> hello (empty string)
> fish 0
> hello fish 6
>
> So you?d be able to do things like `$if{$contains{fish,$query},fish
present,no fishes}`.
>
> Hopefully that makes it more clear.
>
> > Also in query.cc file in omega dir, when defining function
descriptions, what does the evalargs and ensure arguments mean?
>
> 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.)
>
> 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.)
>
> 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!
>
> (Note that it looks to me like there?s a bug in the evalargs handling,
where minargs == N.)
>
> J
>
> --
> James Aylett, occasional trouble-maker
> xapian.org
>
>
--
James Aylett, occasional trouble-maker
xapian.org
Thanks for the feedback! 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. On Mon, Feb 15, 2016 at 5:08 PM, James Aylett <james-xapian at tartarus.org> wrote:> 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.] > > 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.) > > 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 > https://github.com/xapian/xapian/blob/master/xapian-applications/omega/query.cc#L1253 > ). > > 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. > > 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.) > > Github has some documentation on forking and pull requests which may help ( > https://help.github.com/categories/collaborating-on-projects-using-pull-requests/), > but if you have problems, please shout out and someone will try to help! > > J > > > On 15 Feb 2016, at 10:56, Ayush Gupta <ayushgp10 at gmail.com> wrote: > > > > Hi James, > > I have implemented the function here: > https://gist.github.com/ayushgp/fd0b23a6d214e451edf1 > > Please have a look. > > > > > > On Mon, Feb 15, 2016 at 6:11 AM, James Aylett <james-xapian at tartarus.org> > wrote: > > On 14 Feb 2016, at 18:52, Ayush Gupta <ayushgp10 at gmail.com> wrote: > > > > > I tried implementing the bite sized project idea posted here: > https://trac.xapian.org/wiki/ProjectIdeas#AddnewOmegaScriptcommandtodoasubstringsearch > but could not understand what needs to be returned when this command is > called. > > > > 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. > > > > > 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. > > > > Based on the description, here are some examples of what > `$contains{fish,$query}` would expand to based on different values of > `$query`: > > > > $query $contains{fish,$query} > > -------------------------------------- > > hello (empty string) > > fish 0 > > hello fish 6 > > > > So you?d be able to do things like `$if{$contains{fish,$query},fish > present,no fishes}`. > > > > Hopefully that makes it more clear. > > > > > Also in query.cc file in omega dir, when defining function > descriptions, what does the evalargs and ensure arguments mean? > > > > 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.) > > > > 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.) > > > > 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! > > > > (Note that it looks to me like there?s a bug in the evalargs handling, > where minargs == N.) > > > > J > > > > -- > > James Aylett, occasional trouble-maker > > xapian.org > > > > > > -- > James Aylett, occasional trouble-maker > xapian.org > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.xapian.org/pipermail/xapian-devel/attachments/20160215/73557089/attachment.html>
On Mon, Feb 15, 2016 at 12:41:38AM +0000, James Aylett wrote:> 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.)To give a little more context, the reason why we don't parse the query or run the match until we have to is that there are commands which set parameters for query construction and for the match. For example, if you don't want to require AND, OR, etc be written in all capitals, you'd use this near the start of your template: $set{flag_boolean_any_case,1}> (Note that it looks to me like there?s a bug in the evalargs handling, > where minargs == N.)Although it isn't clearly documented, (minargs == N) means just treat all the content as one argument (rather than splitting it at commas). In this case, evalargs is ignored - arguably a bug, but it's a latent one as (minargs == N) is only actually used for comments, where such splitting would be a waste of effort: ${This is a comment, do you see?} Cheers, Olly