You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by NightOwl888 <gi...@git.apache.org> on 2016/08/02 14:39:14 UTC

[GitHub] lucenenet pull request #176: QueryParser

GitHub user NightOwl888 opened a pull request:

    https://github.com/apache/lucenenet/pull/176

    QueryParser

    I have ported much of the QueryParser + the tests.
    
    Current Status
    ====
    
    Most of (about 2/3) of the tests are passing. The good news is that the Classic.QueryParser is passing on all tests (except for one that I was unable to translate).
    
    Not Yet Ported
    ----
    
    1. The entire QueryParser.Flexible namespace.
    2. The [QueryParser.Surround.Query.SrndTruncQuery.VisitMatchingTerms](https://github.com/NightOwl888/lucenenet/blob/queryparser/src/Lucene.Net.QueryParser/Surround/Query/SrndTruncQuery.cs#L87) method.
    3. The [TestDateRange](https://github.com/NightOwl888/lucenenet/blob/queryparser/src/Lucene.Net.Tests.QueryParser/Util/QueryParserTestBase.cs#L751) test for the classic QueryParser.
    
    Points of Interest
    ---
    
    1. The [Locale and Timezone](https://github.com/NightOwl888/lucenenet/blob/queryparser/src/Lucene.Net.QueryParser/Flexible/Standard/CommonQueryParserConfiguration.cs#L77-L85) features of the QueryParser seem a bit incompatible with .NET. The TimeZone property is not actually hooked to anything since there doesn't seem to be a sensible way to use it in .NET. And in .NET we normally set the culture on the current thread or pass it to a method for a one-off. It doesn't make a lot of sense to make it a member of QueryParser, since a single parser instance could be used on different cultures just by switching the culture of the current thread. Thoughts?
    2. I refactored the [QueryParserConstants](https://github.com/NightOwl888/lucenenet/blob/queryparser/src/Lucene.Net.QueryParser/Classic/QueryParserConstants.cs#L20) a bit since it is not possible to add constants to an interface as was done in Java. I weighed the options and ended up splitting them into separate static classes, since there doesn't seem to be a way to reference them in .NET from multiple classes (at least not without a base class) without putting an identifier before the member (`Something.Constant`). I went with `RegexpToken` and `LexicalToken` based on what was in the comments, but feel free to rename them or suggest a different approach.
    3. I think that it would be best to use a `[Flags]` enum in .NET for the [`SimpleQueryParser` constants](https://github.com/NightOwl888/lucenenet/blob/queryparser/src/Lucene.Net.QueryParser/Simple/SimpleQueryParser.cs#L98-L121), since passing these flags to the API as an `int` is not very intuitive. However, currently there are 9 tests failing that all directly involve these constants and I don't want to throw another variable in to the debugging effort. I would be happy to change this as soon as the tests are passing if you agree with this assertion.
    4. I ended up adding a couple of flags to `SimpleQueryParser` to [indicate a "not set" state](https://github.com/NightOwl888/lucenenet/blob/queryparser/src/Lucene.Net.QueryParser/Simple/SimpleQueryParser.cs#L529), since it is not possible to set an enum to null in .NET and IMO a nullable variable type should not be part of a public API.
    5. I have run into a wall with debugging the 9 failing tests on the `SimpleQueryParser`. The [documentation](https://lucene.apache.org/core/4_7_0/queryparser/org/apache/lucene/queryparser/simple/SimpleQueryParser.html) and [comments](https://github.com/apache/lucene-solr/blob/8fdf89690404c0e65784b2c5477552b9dec58591/lucene/queryparser/src/java/org/apache/lucene/queryparser/simple/SimpleQueryParser.java#L102) both indicate that passing a flag is to *enable* the option. However, the [test names](https://github.com/NightOwl888/lucenenet/blob/queryparser/src/Lucene.Net.Tests.QueryParser/Simple/TestSimpleQueryParser.cs#L603-L696) indicate they are testing to *disable* the feature. As a result the wrong type of query is being returned to pass the tests.
    6. I have also run into a wall with the failing tests (which are all of the tests) for the Surround.Parser.QueryParser. For example [this test](https://github.com/NightOwl888/lucenenet/blob/queryparser/src/Lucene.Net.Tests.QueryParser/Surround/Query/Test02Boolean.cs#L128) first correctly identifies the AND token, it then immediately tries to look for a `(` and then blows up because it doesn't exist. I triple checked the code and is doing nothing more than running the initialization code and then doing this check. A little assistance would be appreciated, as I don't have the Java version setup to run to check what it *should* do.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/NightOwl888/lucenenet queryparser

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/lucenenet/pull/176.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #176
    
----
commit 9ecd633c012a6a64af77612bb2b98b3edcb46c02
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-07-30T21:37:39Z

    Added QueryParser project and ported all of the Classic namespace files.

commit f3bc6a363bfecc048c7626b499b105d92a2641c8
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-07-31T09:48:21Z

    Ported tests for the QueryParser.Classic namespace and refactored QueryParserTestBase so the test runner will run all of the tests.

commit a696ce2168c0748edbe0a76b85449ca073aa261b
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-07-31T10:12:18Z

    Refactoring to ensure that the abstract members are available in the context of the base class test (otherwise many of the tests get a NotImplementedException on the QueryParserTestBase).

commit 44f59f1de4ef65ea6c886a1d15b80d576654927e
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-07-31T11:43:14Z

    Changed QueryParserTokenManager line back to the way it was in Lucene.Net 3.0.3, although it differs than the Java version, this appears to be correct in .NET.

commit ea7c7ca2614c21449b7526327b6599c0a04be611
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-07-31T12:02:50Z

    Marked all IncrementToken() methods sealed in test TokenFilter classes.

commit f21ddf259cda41c84359bcfd1aa4324fdb0d7d97
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-07-31T12:15:57Z

    Fixed issue with attributes not being set at the class level in MockCJKSynonymFilter.

commit 30e2532cf0fe4a6ee8724267e1c5ca1130488335
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-07-31T12:20:57Z

    Fixed test name casing.

commit 4a39a91094811adfb9eb1ea1959c68692dde24aa
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-07-31T13:12:20Z

    Fixed long/ulong casting issues in QueryParserTokenManager.

commit fc3501d9111251e9ae8b8144aaec32db12ccd155
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-07-31T14:07:05Z

    Fixed Substring bugs because of the difference between the Java and .NET Substring function.

commit 3013048708ee84a2707677e0216952f4c4a54e08
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-07-31T14:37:32Z

    Fixed bug in Lucene.Net.Util.ToStringUtils that was causing issues with the QueryParser tests. Rolled the Boost(float) method back to the Lucene.Net 3.0.3 state.

commit db3b79bc143adda5df651857bc353a4e5b652eed
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-07-31T18:51:25Z

    Added QueryParser.Analyzing namespace + tests.

commit 3d7dc6dfbe34c696b85b2bfd2201fcaede33224f
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-07-31T20:03:29Z

    Ported QueryParser.ComplexPhrase namespace + tests.

commit a4f15f5d99e31f291c942cfe91d15dabf7d4baf5
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-07-31T22:54:43Z

    Ported QueryParser.Ext namespace + tests.

commit a2df5d1675985be99a828cb9a15ff4f76510c169
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-08-01T08:19:34Z

    Ported QueryParser.Simple namespace + tests.

commit 7337acd6aed52056a33917c8bc8efa5732eae7c2
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-08-02T07:48:31Z

    Ported QueryParser.Surround namespace + tests.

commit 2f63706e8644b050159d3d104cbd508022a950b7
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-08-02T07:48:49Z

    Updated comments.

commit be673c289b7db0b3a4135eef49384db7a6147a9a
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-08-02T12:32:37Z

    Moved Lucene.Net.QueryParser and Lucene.Net.Tests.QueryParser projects into src\ directory.

commit ebfd19618e3646e0860c731617aa78917f3e31b2
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-08-02T11:27:12Z

    Fixed accessibility of classes and members to match that of Java.

commit 0f40417a135055bb9b3dc65c94c381d2fd455ef0
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-08-02T11:32:52Z

    Added missing guard clause to SimpleQueryParser.DefaultOperator.

commit 9e652a0c4e1a0a3f929da56b657b2f40575a6b4c
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-08-02T11:38:01Z

    Added missing documentation to SpanNearClauseFactory

commit 0267b9cd8cab1931a121d2b3e434efb2f723a3b2
Author: Shad Storhaug <sh...@shadstorhaug.com>
Date:   2016-08-02T11:48:04Z

    Removed unnecessary usings.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet pull request #176: Ported QueryParser

Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on a diff in the pull request:

    https://github.com/apache/lucenenet/pull/176#discussion_r73793556
  
    --- Diff: src/Lucene.Net.Core/Util/ToStringUtils.cs ---
    @@ -36,12 +37,13 @@ public static string Boost(float boost)
             {
                 if (boost != 1.0f)
                 {
    -                return "^" + Convert.ToString(boost);
    +                float boostAsLong = (long)boost;
    +                if (boostAsLong == boost)
    +                    return "^" + boost.ToString(".0").Replace(CultureInfo.CurrentCulture.NumberFormat.NumberDecimalSeparator, ".");
    +                return "^" + boost.ToString().Replace(CultureInfo.CurrentCulture.NumberFormat.NumberDecimalSeparator, ".");
    --- End diff --
    
    I copied it over verbatim from Lucene.Net 3.0.3. The previous code was chopping off the .0 on the end, which was different behavior than what was expected by the tests. This was necessary to make some of the tests pass.
    
    For example, 
    
    ```
    Test Name:	TestSimple
    Test FullName:	Lucene.Net.QueryParser.Classic.TestMultiFieldQueryParser.TestSimple
    Test Source:	F:\Projects\lucenenet\src\Lucene.Net.Tests.QueryParser\Classic\TestMultiFieldQueryParser.cs : line 70
    Test Outcome:	Failed
    Test Duration:	0:00:00.003
    
    Result StackTrace:	
    at Lucene.Net.Util.LuceneTestCase.assertEquals(Object expected, Object actual) in F:\Projects\lucenenet\src\Lucene.Net.TestFramework\JavaCompatibility\LuceneTestCase.cs:line 31
    at Lucene.Net.QueryParser.Classic.TestMultiFieldQueryParser.TestSimple() in F:\Projects\lucenenet\src\Lucene.Net.Tests.QueryParser\Classic\TestMultiFieldQueryParser.cs:line 87
    Result Message:	
    Expected string length 33 but was 31. Strings differ at index 16.
      Expected: "((b:one t:one)^2.0) (b:two t:two)"
      But was:  "((b:one t:one)^2) (b:two t:two)"
      ---------------------------^
      ```
    And
    ```
    Test Name:	TestRange
    Test FullName:	Lucene.Net.QueryParser.Util.QueryParserTestBase.TestRange
    Test Source:	F:\Projects\lucenenet\src\Lucene.Net.Tests.QueryParser\Util\QueryParserTestBase.cs : line 657
    Test Outcome:	Failed
    Test Duration:	0:00:00.003
    
    Result StackTrace:	
    at Lucene.Net.Util.LuceneTestCase.fail(String message) in F:\Projects\lucenenet\src\Lucene.Net.TestFramework\JavaCompatibility\LuceneTestCase.cs:line 120
    at Lucene.Net.QueryParser.Util.QueryParserTestBase.AssertQueryEquals(String query, Analyzer a, String result) in F:\Projects\lucenenet\src\Lucene.Net.Tests.QueryParser\Util\QueryParserTestBase.cs:line 204
    at Lucene.Net.QueryParser.Util.QueryParserTestBase.TestRange() in F:\Projects\lucenenet\src\Lucene.Net.Tests.QueryParser\Util\QueryParserTestBase.cs:line 683
    Result Message:	Query /{ a TO z }^2.0/ yielded /{a TO z}^2/, expecting /{a TO z}^2.0/
    ```
    
    Admittedly the code looks bad, but it worked in the prior version and was exactly what was needed to make these (and other) tests green.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #176: Ported QueryParser

Posted by synhershko <gi...@git.apache.org>.
Github user synhershko commented on the issue:

    https://github.com/apache/lucenenet/pull/176
  
    Hi, sorry it took me so long to review. I wanted to review properly and give proper feedback to complement your hard (and very professional!) work here.
    
    So,
    
    1. This makes sense. This is in-line with our .NETification spirit, and as far as we don't remove important functionality I'd say this makes sense and can be removed altogether - or changed to work in a way that makes sense.
    
    2. This makes sense as well. We took this approach in several other places in the code base. I agree the guideline here should be code readability / usability / fluidity.
    
    3. Using an Enum does make sense, and indeed better to wait for tests to stabilize.
    
    4. This might diverge from other places in our codebase. Why is it public API anyway? I can't really see anyone using it, except from people trying to customize the SimpleQueryParser - which is simplistic in it's nature. I'd say let's make it nullable and either accept it's a public API with not much expected usage, or hide it as non-public API. WDYT?
    
    5. From my familiarity with the Java code base, you pass a flag to enable a feature. I think it's just confusing test names, and then a bug somewhere. @uschindler can you confirm? I vaguely recall hitting this one before, will try to look into it too later this week.
    
    6. I will help here. Give me a few (more!) days.
    
    Awesome work dude, thanks very much. I'll leave the decision up to you whether to get this PR merged or still work on it a bit more until it stabilizes. I will look at items 5 and 6 very soon.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #176: Ported QueryParser

Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:

    https://github.com/apache/lucenenet/pull/176
  
    > This might diverge from other places in our codebase. Why is it public API anyway? I can't really see anyone using it, except from people trying to customize the SimpleQueryParser - which is simplistic in it's nature. I'd say let's make it nullable and either accept it's a public API with not much expected usage, or hide it as non-public API. WDYT?
    
    What I meant was, it needs to pass the value through to the public API of another class (I don't recall which offhand). The other class (or classes?) doesn't accept a nullable as part of its public API, so to make it compatible (and do what the Java version does) it would also either need to be nullable, or there would need to be a "guess" at that point in the event it is null. I suppose I could try to figure out what the other class does in the event of a null and move that logic into this class, but that would mean moving business logic from one class into another.
    
    Anyway, can't rule out the possibility that those new flags are part of the problem causing those 9 failing tests. If you have the setup to run in Java to figure out where the execution paths diverge it would be a big help.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet pull request #176: Ported QueryParser

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/lucenenet/pull/176


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #176: Ported QueryParser

Posted by uschindler <gi...@git.apache.org>.
Github user uschindler commented on the issue:

    https://github.com/apache/lucenenet/pull/176
  
    Hi @synhershko,
    
    > From my familiarity with the Java code base, you pass a flag to enable a feature. I think it's just confusing test names, and then a bug somewhere. @uschindler can you confirm? I vaguely recall hitting this one before, will try to look into it too later this week.
    
    I can check Lucene Java's code base later - but I have no idea what's going on. YES - we should also pass a EnumSet in Lucene, but that is old code :-)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #176: Ported QueryParser

Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:

    https://github.com/apache/lucenenet/pull/176
  
    FYI - I have added a commit to get rid of that ugly Lucene.Net 3.0.3 formatting code and added a test to ensure it is the same.
    
    I'm not sure where the right place to drop in a test that is specifically for .NET, but targets untested code from the original version. Should I put the file in the Support directory and set the namespace to Lucene.Net.Util, or will it suffice to leave it in the Util directory?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #176: Ported QueryParser

Posted by synhershko <gi...@git.apache.org>.
Github user synhershko commented on the issue:

    https://github.com/apache/lucenenet/pull/176
  
    This is usually where we put all .NET specific stuff that is required by Core / subprojects: https://github.com/apache/lucenenet/tree/master/src/Lucene.Net.Core/Support/Compatibility
    
    With test stuff here: https://github.com/apache/lucenenet/tree/master/src/Lucene.Net.TestFramework/JavaCompatibility


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #176: Ported QueryParser

Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:

    https://github.com/apache/lucenenet/pull/176
  
    Ok, I have moved the test to Lucene.Net.Tests/core/Support. Apparently there *was* a directory with this name, but it had been removed from the project. Not sure what the reasoning was for this, I suppose this one doesn't belong in the project either if none of those do. But, at least it is now with the rest of the .NET compatibility tests, rather than buried in the Java-ported code.
    
    Feel free to remove it from the project if that is your prerogative. To me it seems like those tests to ensure the parts of Java framework that are ported over are working are a good idea. For example, projects that derive from this one (such as BoboBrowse.Net) might need them, too. But if the implementation is only complete/correct enough to make Lucene.Net work, but not work right for derived libraries, it could mean a lot of copy-paste-edit for those derived libraries.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet pull request #176: Ported QueryParser

Posted by synhershko <gi...@git.apache.org>.
Github user synhershko commented on a diff in the pull request:

    https://github.com/apache/lucenenet/pull/176#discussion_r73792790
  
    --- Diff: src/Lucene.Net.Core/Util/ToStringUtils.cs ---
    @@ -36,12 +37,13 @@ public static string Boost(float boost)
             {
                 if (boost != 1.0f)
                 {
    -                return "^" + Convert.ToString(boost);
    +                float boostAsLong = (long)boost;
    +                if (boostAsLong == boost)
    +                    return "^" + boost.ToString(".0").Replace(CultureInfo.CurrentCulture.NumberFormat.NumberDecimalSeparator, ".");
    +                return "^" + boost.ToString().Replace(CultureInfo.CurrentCulture.NumberFormat.NumberDecimalSeparator, ".");
    --- End diff --
    
    why do you need the Replace part? won't .ToString(CultureInfo) be enough?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Re: [GitHub] lucenenet issue #176: Ported QueryParser

Posted by Itamar Syn-Hershko <it...@code972.com>.
There wasn't one for the 4.8 version, this is what's being ported now

--

Itamar Syn-Hershko
http://code972.com | @synhershko <https://twitter.com/synhershko>
Freelance Developer & Consultant
Lucene.NET committer and PMC member

On Fri, Aug 5, 2016 at 4:20 PM, Benjamin Collins <ag...@gmail.com> wrote:

> There used to be a QueryParser and then it was removed - what happened
> there?
>
> On Tue, Aug 2, 2016 at 4:37 PM, synhershko <gi...@git.apache.org> wrote:
>
> > Github user synhershko commented on the issue:
> >
> >     https://github.com/apache/lucenenet/pull/176
> >
> >     Looks good! will review shortly, thanks!
> >
> >
> > ---
> > If your project is set up for it, you can reply to this email and have
> your
> > reply appear on GitHub as well. If your project does not have this
> feature
> > enabled and wishes so, or if the feature is enabled but not working,
> please
> > contact infrastructure at infrastructure@apache.org or file a JIRA
> ticket
> > with INFRA.
> > ---
> >
>

Re: [GitHub] lucenenet issue #176: Ported QueryParser

Posted by Benjamin Collins <ag...@gmail.com>.
There used to be a QueryParser and then it was removed - what happened
there?

On Tue, Aug 2, 2016 at 4:37 PM, synhershko <gi...@git.apache.org> wrote:

> Github user synhershko commented on the issue:
>
>     https://github.com/apache/lucenenet/pull/176
>
>     Looks good! will review shortly, thanks!
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
>

[GitHub] lucenenet issue #176: Ported QueryParser

Posted by synhershko <gi...@git.apache.org>.
Github user synhershko commented on the issue:

    https://github.com/apache/lucenenet/pull/176
  
    Looks good! will review shortly, thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #176: Ported QueryParser

Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:

    https://github.com/apache/lucenenet/pull/176
  
    Another correction - the Lucene.Net 3.0.3 code had a bug. When passing `0`, it would format the boost as `^.0`, but if you passed `0.123` you would get `^0.123`.
    
    I checked through all of the 4.8 files and there are several unit tests that input `^0.xxx` but nothing boost-related indicates that `^.0` is valid. I think it is safe to assume it should be `^0.0`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #176: Ported QueryParser

Posted by synhershko <gi...@git.apache.org>.
Github user synhershko commented on the issue:

    https://github.com/apache/lucenenet/pull/176
  
    Left one minor comments, looks good to merge otherwise. Thanks for your help!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #176: Ported QueryParser

Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:

    https://github.com/apache/lucenenet/pull/176
  
    Well, everything was going good on this until I got to the Surround parser. If you exclude it like you did with the unfinished parts of Analysis, there are only 12 failing tests. If you also exclude SimpleQueryParser there are only 3 failures. You might consider doing that and rolling those parts of QueryParser and the stable Analysis branch (or perhaps the more complete one) into a pre-release. At least then people will have their old StandardAnalyzer and QueryParser back.
    
    I'll take a look at what can be done about the culture and timezone once it is merged to master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #176: Ported QueryParser

Posted by synhershko <gi...@git.apache.org>.
Github user synhershko commented on the issue:

    https://github.com/apache/lucenenet/pull/176
  
    Merging this before this branch goes too stale. Can you email the dev@ list with all the details here and the TODOs left - and me / others will take it from there?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] lucenenet issue #176: Ported QueryParser

Posted by NightOwl888 <gi...@git.apache.org>.
Github user NightOwl888 commented on the issue:

    https://github.com/apache/lucenenet/pull/176
  
    > This is usually where we put all .NET specific stuff that is required by Core / subprojects: 
    > https://github.com/apache/lucenenet/tree/master/src/Lucene.Net.Core/Support/Compatibility
    
    > With test stuff here: > https://github.com/apache/lucenenet/tree/master/src/Lucene.Net.TestFramework/JavaCompatibility
    
    Hmm... I get that there are extension methods and such in there to prevent having to rewrite too much test code, but if I put the test in `Lucene.Net.TestFramework/JavaCompatibility` it will literally be the *only* executable test in `Lucene.Net.TestFramework`. Seems odd, so just double-checking that is where you want it. But then, there are also no tests currently under `Lucene.Net.Tests/core/Support/`.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---