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:
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.