You are viewing a plain text version of this content. The canonical link for it is here.
Posted to solr-dev@lucene.apache.org by "Eli Levine (JIRA)" <ji...@apache.org> on 2007/10/19 19:39:50 UTC

[jira] Created: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Add confuguration to specify SolrHighlighter implementation
-----------------------------------------------------------

                 Key: SOLR-386
                 URL: https://issues.apache.org/jira/browse/SOLR-386
             Project: Solr
          Issue Type: Improvement
          Components: search
    Affects Versions: 1.3
            Reporter: Eli Levine


It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (SOLR-386) Configurable SolrHighlighter implementation

Posted by "Mike Klaas (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Mike Klaas resolved SOLR-386.
-----------------------------

    Resolution: Fixed

Commited in r639490.  Thanks Tricia!

> Configurable SolrHighlighter implementation
> -------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: highlighter
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>            Assignee: Mike Klaas
>         Attachments: SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch
>
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by "Mike Klaas (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12577163#action_12577163 ] 

Mike Klaas commented on SOLR-386:
---------------------------------

Tricia: thanks for the changes.  I think that your decisions make sense--I'll be sure to followup by the end of the week.

> Add confuguration to specify SolrHighlighter implementation
> -----------------------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: highlighter
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>            Assignee: Mike Klaas
>         Attachments: SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch
>
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by "Eli Levine (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12571153#action_12571153 ] 

Eli Levine commented on SOLR-386:
---------------------------------

I looked at the patch and it seems it does what I need, which is simply to allow the class of SolrHighlighter to be configurable.  Thanks, Tricia.

Eli

> Add confuguration to specify SolrHighlighter implementation
> -----------------------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: highlighter
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>            Assignee: Mike Klaas
>         Attachments: SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch
>
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by "Eli Levine (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12536348 ] 

Eli Levine commented on SOLR-386:
---------------------------------

This would be useful for projects with non-standard highlighting needs.  In our case, under certain circumstances we need to run a field through the highlighter multiple times using different fragmenters if previous runs did not produce any highlighted fragments.  Also, this would allow us to use SpanScorer for phrase highlighting, as it is instantiated differently from other scorers (needs CachingTokenFilter, etc.)

Since the Solr highlighter is a plug-in, it seems natural to allow different implementations to be used, similar to what's allowed for request and update handlers and other configurable parts of SolrCore.

Thanks,

Eli


> Add confuguration to specify SolrHighlighter implementation
> -----------------------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: search
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by "Tricia Williams (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tricia Williams updated SOLR-386:
---------------------------------

    Attachment: SOLR-386-SolrHighlighter.patch

This patch allows highlighting to be plugged in.

What I did:
 * Made SolrHighlighter an interface
 * The old SolrHighlighter became DefaultSolrHighlighter
 * Instantiate the highlighter in SolrCore based on what is in the solrconfig.xml

So to roll your own
 * Implement SolrHighlighter (ie org.apache.solr.highlight.MySolrHighlighter)
 * find <highlighting> in solrconfig.xml and modify to <highlighting class="org.apache.solr.highlight.MySolrHighlighter">

This patch builds on changes made to trunk by SOLR-281.  This patch also contains these changes (meaning you should apply this patch to the trunk).  I get the feeling that this is probably not the right way to build a dependent patch, but I don't know any better.  Let me know if I should change how I built this patch.

> Add confuguration to specify SolrHighlighter implementation
> -----------------------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: highlighter
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>         Attachments: SOLR-386-SolrHighlighter.patch
>
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Re: [jira] Commented: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by Tricia Williams <wi...@gmail.com>.
OK.  So I think I fixed the whitespace problem. 

Thanks for explaining the problem with interfaces -- that makes sense 
now.  I assume that EventListener and RequestHandler are interfaces 
because they've been thought long and hard about and are not going to 
change?

My first try at the patch was just to include the public methods, which 
are the ones you list:
> .initialize(Config)
> .isHighlightEnabled(SolrParams)
> .doHighlighting(...)
> .getHighlightFields(...) 
I discovered that the unit tests call the formatters and fragmenters 
directly so in the interface version I made public get methods for 
these.  Now that we're using an abstract class I am able to just include 
these as is - so no changes to HighlighterTest this time.  But speaking 
of unit tests... Anecdotally I know that the SolrCore changes allow the 
highlighter to be configured (my custom highlighter).  I guess I should 
write a unit test that ensures this works.  I'll do that now.

After doing some thinking I decided to leave the default implementation 
of isHighlightingEnabled(SolrParams), and getHighlightFields(...) in the 
abstract class because both methods deal with reading parameters.  I 
can't think of a use case of a highlighter that wouldn't use this or at 
worst ignore/override this method.  Is this a reasonable decision?

I wasn't sure what to do with the logger, so I left it in the 
AbstractSolrHighlighter.

Thoughts?
Tricia

p.s.  I've attached the updated patch since JIRA appears to be down.

Mike Klaas wrote:
> Hi Tricia,
>
> I don't suggest removing whitespace by hand--the best way to avoid the 
> situation is to make sure that your editor does not automatically edit 
> the whitespace of a file (some good-intentioned editors do this).
>
> The maintenance concern with interfaces is that if we want to add a 
> method to the highlighter contract, doing so will break all custom 
> implementors of the interface.  If it is an abstract class that must 
> be subclassed, we can add a default implementation to the new method 
> without breaking everyone's code.
>
> As for the contract, I was going to review the proposed patch to see 
> what the interface consists of, but I think that all that is required is:
>
> .initialize(Config)
> .isHighlightEnabled(SolrParams)
> .doHighlighting(...)
> .getHighlightFields(...)
>
> (which is probably what you had in your patch already---JIRA seems 
> down at the moment so I can't check).
>
> cheers,
> -Mike
>
> On 5-Mar-08, at 2:49 PM, Tricia Williams wrote:
>
>> Thanks to Grant and Mike for the feedback!  It is much appreciated.  
>> Is there a quick and easy way to check for unnecessary whitespace 
>> changes?  It isn't that hard for me to go through the patch by hand 
>> to find and remove them, but if there is an easier way I'm happy to 
>> hear it.
>>
>> I had taken the suggestion that Eli gave somewhat literally and made 
>> SolrHighlighter an interface like RequestHandler.  In the SolrCore 
>> there are three existing objects that are configured: 
>> SolrEventListener, SolrRequestHandler, and UpdateHandler.  The first 
>> two are interfaces and the third is an abstract class.  While I'm 
>> sure the maintenance concern is legitimate, I'm not sure what the 
>> maintenance concern is - could someone elaborate?
>>
>> I'll take your advice and make an AbstractSolrHighlighter that 
>> SolrHighlighter (and my custom highlighter) extends.  I noticed that 
>> some of the other configurable objects implement SolrInfoMBean.  Is 
>> this something that the SolrHighlighter/AbstractSolrHighlighter 
>> should also do?
>>
>> Thanks,
>> Tricia
>>
>> Mike Klaas (JIRA) wrote:
>>>    [ 
>>> https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12574337#action_12574337 
>>> ]
>>> Mike Klaas commented on SOLR-386:
>>> ---------------------------------
>>>
>>> Hi Tricia,
>>>
>>> I'm not sure that I would ever use SolrHighlighter overriding (if I 
>>> had the need, I probably would just make a separate search 
>>> component).  However, several people want this functionality and it 
>>> has relatively low implementation/maintenance cost.
>>>
>>> There are a few things that need to be done to get the patch 
>>> committed.  First, the unnecessary whitespace changes in SolrCore 
>>> shouldn't be in the diff (it makes it really hard to see the 
>>> changes, and might make it hard to apply/revert).  Also, I'm 
>>> skeptical of using interfaces for this type of thing, for 
>>> maintenance reasons.  Perhaps we could move to one of the approaches 
>>> that Grant suggested?
>>>
>>> Thanks again for the contribution, and sorry it took so long
>>>
>>>
>>>
>>>
>>>> Add confuguration to specify SolrHighlighter implementation
>>>> -----------------------------------------------------------
>>>>
>>>>                Key: SOLR-386
>>>>                URL: https://issues.apache.org/jira/browse/SOLR-386
>>>>            Project: Solr
>>>>         Issue Type: Improvement
>>>>         Components: highlighter
>>>>   Affects Versions: 1.3
>>>>           Reporter: Eli Levine
>>>>           Assignee: Mike Klaas
>>>>        Attachments: SOLR-386-SolrHighlighter.patch, 
>>>> SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, 
>>>> SOLR-386-SolrHighlighter.patch
>>>>
>>>>
>>>> It would be great if SolrCore allowed the highlighter class to be 
>>>> configurable.  A good way would be to add a +class+ attribute to 
>>>> the <highlighting> element in solrconfig.xml, similar to how the 
>>>> RequestHandler instance is initialized in SorCore.
>>>>
>>>
>>>
>>
>
>


Re: [jira] Commented: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by Mike Klaas <mi...@gmail.com>.
Hi Tricia,

I don't suggest removing whitespace by hand--the best way to avoid the  
situation is to make sure that your editor does not automatically edit  
the whitespace of a file (some good-intentioned editors do this).

The maintenance concern with interfaces is that if we want to add a  
method to the highlighter contract, doing so will break all custom  
implementors of the interface.  If it is an abstract class that must  
be subclassed, we can add a default implementation to the new method  
without breaking everyone's code.

As for the contract, I was going to review the proposed patch to see  
what the interface consists of, but I think that all that is required  
is:

.initialize(Config)
.isHighlightEnabled(SolrParams)
.doHighlighting(...)
.getHighlightFields(...)

(which is probably what you had in your patch already---JIRA seems  
down at the moment so I can't check).

cheers,
-Mike

On 5-Mar-08, at 2:49 PM, Tricia Williams wrote:

> Thanks to Grant and Mike for the feedback!  It is much appreciated.   
> Is there a quick and easy way to check for unnecessary whitespace  
> changes?  It isn't that hard for me to go through the patch by hand  
> to find and remove them, but if there is an easier way I'm happy to  
> hear it.
>
> I had taken the suggestion that Eli gave somewhat literally and made  
> SolrHighlighter an interface like RequestHandler.  In the SolrCore  
> there are three existing objects that are configured:  
> SolrEventListener, SolrRequestHandler, and UpdateHandler.  The first  
> two are interfaces and the third is an abstract class.  While I'm  
> sure the maintenance concern is legitimate, I'm not sure what the  
> maintenance concern is - could someone elaborate?
>
> I'll take your advice and make an AbstractSolrHighlighter that  
> SolrHighlighter (and my custom highlighter) extends.  I noticed that  
> some of the other configurable objects implement SolrInfoMBean.  Is  
> this something that the SolrHighlighter/AbstractSolrHighlighter  
> should also do?
>
> Thanks,
> Tricia
>
> Mike Klaas (JIRA) wrote:
>>    [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12574337 
>> #action_12574337 ]
>> Mike Klaas commented on SOLR-386:
>> ---------------------------------
>>
>> Hi Tricia,
>>
>> I'm not sure that I would ever use SolrHighlighter overriding (if I  
>> had the need, I probably would just make a separate search  
>> component).  However, several people want this functionality and it  
>> has relatively low implementation/maintenance cost.
>>
>> There are a few things that need to be done to get the patch  
>> committed.  First, the unnecessary whitespace changes in SolrCore  
>> shouldn't be in the diff (it makes it really hard to see the  
>> changes, and might make it hard to apply/revert).  Also, I'm  
>> skeptical of using interfaces for this type of thing, for  
>> maintenance reasons.  Perhaps we could move to one of the  
>> approaches that Grant suggested?
>>
>> Thanks again for the contribution, and sorry it took so long
>>
>>
>>
>>
>>> Add confuguration to specify SolrHighlighter implementation
>>> -----------------------------------------------------------
>>>
>>>                Key: SOLR-386
>>>                URL: https://issues.apache.org/jira/browse/SOLR-386
>>>            Project: Solr
>>>         Issue Type: Improvement
>>>         Components: highlighter
>>>   Affects Versions: 1.3
>>>           Reporter: Eli Levine
>>>           Assignee: Mike Klaas
>>>        Attachments: SOLR-386-SolrHighlighter.patch, SOLR-386- 
>>> SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386- 
>>> SolrHighlighter.patch
>>>
>>>
>>> It would be great if SolrCore allowed the highlighter class to be  
>>> configurable.  A good way would be to add a +class+ attribute to  
>>> the <highlighting> element in solrconfig.xml, similar to how the  
>>> RequestHandler instance is initialized in SorCore.
>>>
>>
>>
>


Re: [jira] Commented: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by Tricia Williams <wi...@gmail.com>.
Thanks to Grant and Mike for the feedback!  It is much appreciated.  Is 
there a quick and easy way to check for unnecessary whitespace changes?  
It isn't that hard for me to go through the patch by hand to find and 
remove them, but if there is an easier way I'm happy to hear it.

I had taken the suggestion that Eli gave somewhat literally and made 
SolrHighlighter an interface like RequestHandler.  In the SolrCore there 
are three existing objects that are configured: SolrEventListener, 
SolrRequestHandler, and UpdateHandler.  The first two are interfaces and 
the third is an abstract class.  While I'm sure the maintenance concern 
is legitimate, I'm not sure what the maintenance concern is - could 
someone elaborate?

I'll take your advice and make an AbstractSolrHighlighter that 
SolrHighlighter (and my custom highlighter) extends.  I noticed that 
some of the other configurable objects implement SolrInfoMBean.  Is this 
something that the SolrHighlighter/AbstractSolrHighlighter should also do?

Thanks,
Tricia

Mike Klaas (JIRA) wrote:
>     [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12574337#action_12574337 ] 
>
> Mike Klaas commented on SOLR-386:
> ---------------------------------
>
> Hi Tricia,
>
> I'm not sure that I would ever use SolrHighlighter overriding (if I had the need, I probably would just make a separate search component).  However, several people want this functionality and it has relatively low implementation/maintenance cost.
>
> There are a few things that need to be done to get the patch committed.  First, the unnecessary whitespace changes in SolrCore shouldn't be in the diff (it makes it really hard to see the changes, and might make it hard to apply/revert).  Also, I'm skeptical of using interfaces for this type of thing, for maintenance reasons.  Perhaps we could move to one of the approaches that Grant suggested?
>
> Thanks again for the contribution, and sorry it took so long
>
>
>
>   
>> Add confuguration to specify SolrHighlighter implementation
>> -----------------------------------------------------------
>>
>>                 Key: SOLR-386
>>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>>             Project: Solr
>>          Issue Type: Improvement
>>          Components: highlighter
>>    Affects Versions: 1.3
>>            Reporter: Eli Levine
>>            Assignee: Mike Klaas
>>         Attachments: SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch
>>
>>
>> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.
>>     
>
>   


[jira] Commented: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by "Mike Klaas (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12574337#action_12574337 ] 

Mike Klaas commented on SOLR-386:
---------------------------------

Hi Tricia,

I'm not sure that I would ever use SolrHighlighter overriding (if I had the need, I probably would just make a separate search component).  However, several people want this functionality and it has relatively low implementation/maintenance cost.

There are a few things that need to be done to get the patch committed.  First, the unnecessary whitespace changes in SolrCore shouldn't be in the diff (it makes it really hard to see the changes, and might make it hard to apply/revert).  Also, I'm skeptical of using interfaces for this type of thing, for maintenance reasons.  Perhaps we could move to one of the approaches that Grant suggested?

Thanks again for the contribution, and sorry it took so long



> Add confuguration to specify SolrHighlighter implementation
> -----------------------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: highlighter
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>            Assignee: Mike Klaas
>         Attachments: SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch
>
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by "Tricia Williams (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tricia Williams updated SOLR-386:
---------------------------------

    Attachment: SOLR-386-SolrHighlighter.patch

SOLR-281 was recently commited.  Formerly those changes were also included in this patch.  The changes made by that patch have been removed from this patch so SOLR-386-SolrHighlighter.patch applies cleanly to trunk again.  Unless someone has comments, or an alternate implementation I don't foresee this patch changing again.

> Add confuguration to specify SolrHighlighter implementation
> -----------------------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: highlighter
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>         Attachments: SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch
>
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by "Ryan McKinley (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12536293 ] 

Ryan McKinley commented on SOLR-386:
------------------------------------

How is this different from SOLR-225?

The example solrconfig.xml in trunk includes:


  <highlighting>
   <!-- Configure the standard fragmenter -->
   <!-- This could most likely be commented out in the "default" case -->
   <fragmenter name="gap" class="org.apache.solr.highlight.GapFragmenter" default="true">
    <lst name="defaults">
     <int name="hl.fragsize">100</int>
    </lst>
   </fragmenter>

   <!-- A regular-expression-based fragmenter (f.i., for sentence extraction) -->
   <fragmenter name="regex" class="org.apache.solr.highlight.RegexFragmenter">
    <lst name="defaults">
      <!-- slightly smaller fragsizes work better because of slop -->
      <int name="hl.fragsize">70</int>
      <!-- allow 50% slop on fragment sizes -->
      <float name="hl.regex.slop">0.5</float> 
      <!-- a basic sentence pattern -->
      <str name="hl.regex.pattern">[-\w ,/\n\"']{20,200}</str>
    </lst>
   </fragmenter>
   
   <!-- Configure the standard formatter -->
   <formatter name="html" class="org.apache.solr.highlight.HtmlFormatter" default="true">
    <lst name="defaults">
     <str name="hl.simple.pre"><![CDATA[<em>]]></str>
     <str name="hl.simple.post"><![CDATA[</em>]]></str>
    </lst>
   </formatter>
  </highlighting>


> Add confuguration to specify SolrHighlighter implementation
> -----------------------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: search
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by "Eli Levine (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12536295 ] 

Eli Levine commented on SOLR-386:
---------------------------------

This is different in that this request asks for the SolrHighlighter implementation itself to be pluggable, not just fragmenters and formatters.

> Add confuguration to specify SolrHighlighter implementation
> -----------------------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: search
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by "Tricia Williams (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tricia Williams updated SOLR-386:
---------------------------------

    Attachment: SOLR-386-SolrHighlighter.patch

I'd really like some feedback on this patch.  I've just updated the patch based on changes that have been made to SolrHighlighter.java since revision 594314).

Eli, does this meet your needs?  This is all I need in SOLR-380 to plug in a custom highlighter.  I would really appreciate if this could be committed by someone so that I can stop worrying about keeping up with revisions.  It has been assigned to Mike Klass so his feedback in particular would be valuable to me.

Thanks,
Tricia

> Add confuguration to specify SolrHighlighter implementation
> -----------------------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: highlighter
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>            Assignee: Mike Klaas
>         Attachments: SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch
>
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by "Tricia Williams (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tricia Williams updated SOLR-386:
---------------------------------

    Attachment: SOLR-386-SolrHighlighter.patch

Updated patch to work with recent changes made to SolrCore.  Should apply against a clean trunk again.  No further changes.

> Add confuguration to specify SolrHighlighter implementation
> -----------------------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: highlighter
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>         Attachments: SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch
>
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by "Mike Klaas (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Mike Klaas updated SOLR-386:
----------------------------

    Component/s:     (was: search)
                 highlighter

> Add confuguration to specify SolrHighlighter implementation
> -----------------------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: highlighter
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by "Mike Klaas (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12536302 ] 

Mike Klaas commented on SOLR-386:
---------------------------------

I suppose the real question is What behaviour are you trying to achieve by ripping out the whole highlighter implementation?  If you have custom code that does something completely different, it is probably easier to just call that code manually from your own request handler.

> Add confuguration to specify SolrHighlighter implementation
> -----------------------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: search
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-386) Configurable SolrHighlighter implementation

Posted by "Mike Klaas (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Mike Klaas updated SOLR-386:
----------------------------

    Summary: Configurable SolrHighlighter implementation  (was: Add confuguration to specify SolrHighlighter implementation)

> Configurable SolrHighlighter implementation
> -------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: highlighter
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>            Assignee: Mike Klaas
>         Attachments: SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch
>
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Assigned: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by "Mike Klaas (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Mike Klaas reassigned SOLR-386:
-------------------------------

    Assignee: Mike Klaas

> Add confuguration to specify SolrHighlighter implementation
> -----------------------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: highlighter
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>            Assignee: Mike Klaas
>         Attachments: SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch
>
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by "Tricia Williams (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tricia Williams updated SOLR-386:
---------------------------------

    Attachment: SOLR-386-SolrHighlighter.patch

OK.  So I think I fixed the whitespace problem.

Thanks for explaining the problem with interfaces -- that makes sense now.  I assume that EventListener and RequestHandler are interfaces because they've been thought long and hard about and are not going to change?

My first try at the patch was just to include the public methods, which are the ones you (MIke Klaas) list:
> .initialize(Config)
> .isHighlightEnabled(SolrParams)
> .doHighlighting(...)
> .getHighlightFields(...) 

I discovered that the unit tests call the formatters and fragmenters directly so in the interface version I had made public get methods for these.  Now that we're using an abstract class I am able to just include these as is - so no changes to HighlighterTest this time.  But speaking of unit tests... Anecdotally I know that the SolrCore changes allow the highlighter to be configured (my custom highlighter).  I wrote HighlighterConfigTest as a unit test for this functionality.

I decided to leave the default implementation of isHighlightingEnabled(SolrParams), and getHighlightFields(...) in the abstract class because both methods deal with reading parameters.  I can't think of a use case of a highlighter that wouldn't use this or at worst ignore/override this method.  Is this a reasonable decision?

I wasn't sure what to do with the logger, so I left it in the AbstractSolrHighlighter.  This decision is based on the example of UpdateHandler. 

Hmm... I just realized that naming the abstraction of SolrHighlighter AbstractSolrHighlighter causes problems all over the code when you want your custom highlighter to plugin.  I think the path of least resistance is to call the abstract class SolrHighlighter and the existing implementation DefaultSolrHighlighter.

Thoughts?

> Add confuguration to specify SolrHighlighter implementation
> -----------------------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: highlighter
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>            Assignee: Mike Klaas
>         Attachments: SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch
>
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SOLR-386) Add confuguration to specify SolrHighlighter implementation

Posted by "Grant Ingersoll (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SOLR-386?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12573314#action_12573314 ] 

Grant Ingersoll commented on SOLR-386:
--------------------------------------

{quote}
Made SolrHighlighter an interface
{quote}

Why not just extend and override?  The general problem w/ interfaces is it gets hard to maintain back-compatibility in this kind of environment.  Another option would be to abstract SolrHighlighter to extend AbstractSolrHighlighter and then your new highlighter could just extend AbstractSolrHightlighter

> Add confuguration to specify SolrHighlighter implementation
> -----------------------------------------------------------
>
>                 Key: SOLR-386
>                 URL: https://issues.apache.org/jira/browse/SOLR-386
>             Project: Solr
>          Issue Type: Improvement
>          Components: highlighter
>    Affects Versions: 1.3
>            Reporter: Eli Levine
>            Assignee: Mike Klaas
>         Attachments: SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch, SOLR-386-SolrHighlighter.patch
>
>
> It would be great if SolrCore allowed the highlighter class to be configurable.  A good way would be to add a +class+ attribute to the <highlighting> element in solrconfig.xml, similar to how the RequestHandler instance is initialized in SorCore.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.