[jira] [Commented] (LUCENE-4896) PostingsHighlighter should use a interface of PassageFormatter instead of a class
[
https://issues.apache.org/jira/browse/LUCENE-4896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13630374#comment-13630374
]
Robert Muir commented on LUCENE-4896:
-
As i mentioned earlier i think PassageFormatter should be an abstract class
(not an interface) with just that one method currently:
{code}
public String format(Passage passages[], String content);
{code}
But it might need a more complex API later, e.g. to support LUCENE-4906. So i
think an abstract class is the right way to go (in addition to the reasons i
mentioned in my first comment)
opening up the DefaultPassageFormatter looks good in the patch. i think making
append protected is fine. In general though its just useful if someone wants to
tweak the default implementation. if their impl is radically different they
should just be extending PassageFormatter (the abstract one).
PostingsHighlighter should use a interface of PassageFormatter instead of a
class
-
Key: LUCENE-4896
URL: https://issues.apache.org/jira/browse/LUCENE-4896
Project: Lucene - Core
Issue Type: Improvement
Components: modules/highlighter
Affects Versions: 4.2
Environment: NA
Reporter: Sebastien Dionne
Labels: newdev
Attachments: LUCENE-4896.patch
In my project I need a custom PassageFormatter to use with
PostingsHighlighter. I extended PassageFormatter to override format(...)
but if I do that, I don't have access to the private variables. So instead
of changing the scope to protected, it should be more usefull to use a
interface for PassageFormatter.
like public DefaultPassageFormatter implements PassageFormatter.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-4896) PostingsHighlighter should use a interface of PassageFormatter instead of a class
[ https://issues.apache.org/jira/browse/LUCENE-4896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13630383#comment-13630383 ] Robert Muir commented on LUCENE-4896: - One other thing: in the DefaultPassageFormatter we should add @Override annotation to format(), and remove the javadocs block completely: it will be inherited from PassageFormatter. I'd also add lucene.experimental annotation to the abstract PassageFormatter class javadocs. PostingsHighlighter should use a interface of PassageFormatter instead of a class - Key: LUCENE-4896 URL: https://issues.apache.org/jira/browse/LUCENE-4896 Project: Lucene - Core Issue Type: Improvement Components: modules/highlighter Affects Versions: 4.2 Environment: NA Reporter: Sebastien Dionne Labels: newdev Attachments: LUCENE-4896.patch In my project I need a custom PassageFormatter to use with PostingsHighlighter. I extended PassageFormatter to override format(...) but if I do that, I don't have access to the private variables. So instead of changing the scope to protected, it should be more usefull to use a interface for PassageFormatter. like public DefaultPassageFormatter implements PassageFormatter. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-4896) PostingsHighlighter should use a interface of PassageFormatter instead of a class
[ https://issues.apache.org/jira/browse/LUCENE-4896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13630392#comment-13630392 ] Luca Cavanna commented on LUCENE-4896: -- Just read your last comment, going to add another patch :) PostingsHighlighter should use a interface of PassageFormatter instead of a class - Key: LUCENE-4896 URL: https://issues.apache.org/jira/browse/LUCENE-4896 Project: Lucene - Core Issue Type: Improvement Components: modules/highlighter Affects Versions: 4.2 Environment: NA Reporter: Sebastien Dionne Labels: newdev Attachments: LUCENE-4896.patch, LUCENE-4896.patch In my project I need a custom PassageFormatter to use with PostingsHighlighter. I extended PassageFormatter to override format(...) but if I do that, I don't have access to the private variables. So instead of changing the scope to protected, it should be more usefull to use a interface for PassageFormatter. like public DefaultPassageFormatter implements PassageFormatter. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-4896) PostingsHighlighter should use a interface of PassageFormatter instead of a class
[ https://issues.apache.org/jira/browse/LUCENE-4896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13630411#comment-13630411 ] Robert Muir commented on LUCENE-4896: - Patch looks good... I'm going to apply it and review one last time. Thanks Luca! Just to try to explain my reasoning again for the abstract class: in general to me if its the class's primary purpose then its the correct way from an inheritance perspective. Whereas interfaces (e.g. Closeable) are not. This way we get the possibility of incorporating default behavior and things like this. One reason why this could be useful, when we look at issues like LUCENE-4906, we might want to make formatter more powerful to do this: maybe it has a lower-level method that e.g. gets docids and all kinds of other things, or even all the passages for all the top-docs. The default implementation could still call format() for each doc, but that would allow someone to e.g. build a complex response structure for the whole top-docs at once or something like that. I'm not arguing that would necessarily be a good API at all, its just a theoretical example that wouldnt work well with interfaces :) PostingsHighlighter should use a interface of PassageFormatter instead of a class - Key: LUCENE-4896 URL: https://issues.apache.org/jira/browse/LUCENE-4896 Project: Lucene - Core Issue Type: Improvement Components: modules/highlighter Affects Versions: 4.2 Environment: NA Reporter: Sebastien Dionne Labels: newdev Attachments: LUCENE-4896.patch, LUCENE-4896.patch, LUCENE-4896.patch In my project I need a custom PassageFormatter to use with PostingsHighlighter. I extended PassageFormatter to override format(...) but if I do that, I don't have access to the private variables. So instead of changing the scope to protected, it should be more usefull to use a interface for PassageFormatter. like public DefaultPassageFormatter implements PassageFormatter. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-4896) PostingsHighlighter should use a interface of PassageFormatter instead of a class
[ https://issues.apache.org/jira/browse/LUCENE-4896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13630441#comment-13630441 ] Luca Cavanna commented on LUCENE-4896: -- I see what you mean Robert, thanks a lot for your explanation. I would have probably ended up with an interface + abstract class then ;) Let's see what I can come up with for LUCENE-4906... PostingsHighlighter should use a interface of PassageFormatter instead of a class - Key: LUCENE-4896 URL: https://issues.apache.org/jira/browse/LUCENE-4896 Project: Lucene - Core Issue Type: Improvement Components: modules/highlighter Affects Versions: 4.2 Environment: NA Reporter: Sebastien Dionne Labels: newdev Attachments: LUCENE-4896.patch, LUCENE-4896.patch, LUCENE-4896.patch In my project I need a custom PassageFormatter to use with PostingsHighlighter. I extended PassageFormatter to override format(...) but if I do that, I don't have access to the private variables. So instead of changing the scope to protected, it should be more usefull to use a interface for PassageFormatter. like public DefaultPassageFormatter implements PassageFormatter. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-4896) PostingsHighlighter should use a interface of PassageFormatter instead of a class
[ https://issues.apache.org/jira/browse/LUCENE-4896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13630444#comment-13630444 ] Robert Muir commented on LUCENE-4896: - Thanks again Luca. I committed this with only one change to your patch (i added some javadocs to append, since its now protected). I'm backporting now and then i'll resolve the issue. PostingsHighlighter should use a interface of PassageFormatter instead of a class - Key: LUCENE-4896 URL: https://issues.apache.org/jira/browse/LUCENE-4896 Project: Lucene - Core Issue Type: Improvement Components: modules/highlighter Affects Versions: 4.2 Environment: NA Reporter: Sebastien Dionne Labels: newdev Attachments: LUCENE-4896.patch, LUCENE-4896.patch, LUCENE-4896.patch In my project I need a custom PassageFormatter to use with PostingsHighlighter. I extended PassageFormatter to override format(...) but if I do that, I don't have access to the private variables. So instead of changing the scope to protected, it should be more usefull to use a interface for PassageFormatter. like public DefaultPassageFormatter implements PassageFormatter. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-4896) PostingsHighlighter should use a interface of PassageFormatter instead of a class
[ https://issues.apache.org/jira/browse/LUCENE-4896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13622199#comment-13622199 ] Robert Muir commented on LUCENE-4896: - [~survivant] Do you want to create a patch for this issue? PostingsHighlighter should use a interface of PassageFormatter instead of a class - Key: LUCENE-4896 URL: https://issues.apache.org/jira/browse/LUCENE-4896 Project: Lucene - Core Issue Type: Improvement Components: modules/highlighter Affects Versions: 4.2 Environment: NA Reporter: Sebastien Dionne In my project I need a custom PassageFormatter to use with PostingsHighlighter. I extended PassageFormatter to override format(...) but if I do that, I don't have access to the private variables. So instead of changing the scope to protected, it should be more usefull to use a interface for PassageFormatter. like public DefaultPassageFormatter implements PassageFormatter. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[jira] [Commented] (LUCENE-4896) PostingsHighlighter should use a interface of PassageFormatter instead of a class
[ https://issues.apache.org/jira/browse/LUCENE-4896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13620984#comment-13620984 ] Robert Muir commented on LUCENE-4896: - I agree there are a few bugs here: # we should split PassageFormatter (abstract) from its default implementation. # the default implementation should expose its params as protected, so its still extensible. However I don't think an interface is best for this one: formatting is the key thing this class will do (as opposed to e.g. closeable). So I think it should be an abstract class, even if today its api is only one method, i expect this API might unfortunately grow larger :) PostingsHighlighter should use a interface of PassageFormatter instead of a class - Key: LUCENE-4896 URL: https://issues.apache.org/jira/browse/LUCENE-4896 Project: Lucene - Core Issue Type: Improvement Components: modules/highlighter Affects Versions: 4.2 Environment: NA Reporter: Sebastien Dionne In my project I need a custom PassageFormatter to use with PostingsHighlighter. I extended PassageFormatter to override format(...) but if I do that, I don't have access to the private variables. So instead of changing the scope to protected, it should be more usefull to use a interface for PassageFormatter. like public DefaultPassageFormatter implements PassageFormatter. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
