Lucene.Net
  1. Lucene.Net
  2. LUCENENET-426

Mark BaseFragmentsBuilder methods as virtual

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: Lucene.Net 2.9.2, Lucene.Net 2.9.4, Lucene.Net 2.9.4g, Lucene.Net 3.0.3
    • Component/s: Lucene.Net Contrib
    • Labels:
      None

      Description

      Without marking methods in BaseFragmentsBuilder as virtual, it is meaningless to have FragmentsBuilder deriving from a class named "Base", since most of its functionality cannot be overridden. Attached is a patch for marking the important methods virtual.

      1. fvh.patch
        2 kB
        Itamar Syn-Hershko

        Activity

        Hide
        Itamar Syn-Hershko added a comment -

        Patch fixing this

        Show
        Itamar Syn-Hershko added a comment - Patch fixing this
        Hide
        Digy added a comment -

        Thanks Itamar.
        Fixed in trunk & 2.9.4g branch.

        DIGY

        Show
        Digy added a comment - Thanks Itamar. Fixed in trunk & 2.9.4g branch. DIGY
        Hide
        Itamar Syn-Hershko added a comment -

        Apparently that was not enough. I hit a need to override this one too:

        protected Field[] GetFields(IndexReader reader, int docId, String fieldName)

        Perhaps it'd make sense to make all protected virtual? In Java you can override anything that is not final, so that would be compatible with the original version.

        Show
        Itamar Syn-Hershko added a comment - Apparently that was not enough. I hit a need to override this one too: protected Field[] GetFields(IndexReader reader, int docId, String fieldName) Perhaps it'd make sense to make all protected virtual? In Java you can override anything that is not final, so that would be compatible with the original version.
        Hide
        Digy added a comment -

        Perhaps you do something wrong
        Since you posted in your blog as

        Perhaps you have tackled this before: you wanted to use Lucene's FastVectorHighlighter (aka FVH), but since you have a custom CharFilter in your analysis chain, the highlighter fails to produce valid fragments.

        which is definitely wrong.

        DIGY

        Show
        Digy added a comment - Perhaps you do something wrong Since you posted in your blog as Perhaps you have tackled this before: you wanted to use Lucene's FastVectorHighlighter (aka FVH), but since you have a custom CharFilter in your analysis chain, the highlighter fails to produce valid fragments. which is definitely wrong. DIGY
        Hide
        Itamar Syn-Hershko added a comment - - edited

        I think we already went through this?

        Anyway, I don't see how that's relevant. My point here is language difference you didn't take into account, and that point is valid.

        As for my particular use case - I'm trying to highlight based on termvectors of field A using the stored content from field B. I don't think I'm doing anything wrong here. Only spent 10 minutes on that tho.

        Show
        Itamar Syn-Hershko added a comment - - edited I think we already went through this? Anyway, I don't see how that's relevant. My point here is language difference you didn't take into account, and that point is valid. As for my particular use case - I'm trying to highlight based on termvectors of field A using the stored content from field B. I don't think I'm doing anything wrong here. Only spent 10 minutes on that tho.
        Hide
        Digy added a comment -

        10 min. work done.
        DIGY

        Show
        Digy added a comment - 10 min. work done. DIGY

          People

          • Assignee:
            Unassigned
            Reporter:
            Itamar Syn-Hershko
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development