Mike, I've only had a chance to skim your patch so far, but on the
surface it looks very good to me. One little thing scares me a little
bit...
: 5. SolrPluginUtils.getParam() only uses the default parameter if it is null,
not blank.
...in principal, I agree with the sentiment that "" should be a legal
value, but the Utils class was implimented this way to support URLs
generated by HTML form submission, where the param is still included in
the URL but with an empty value. Changing this would break at least one
or two Solr uses I know of.
Then again, if we want to change it, now would be better then latter
... hmmm. Perhaps an alternate approach might be to continue to use the
default when the param is empty (""), but stop trimming the values before
the comparison, so that " " is a legal value that prevents the default
form being used?
...this is of course a minor bit of paranoia on my part, and
should not detract from the coolness of the rest of the patch.
I'm eager to see what other people on the list (who know more about
highlighting and are interested in using highlighting with Solr) think
about your patch ... alas, Erik seems to be quite busy right now, maybe he
can share some thoughts next week? (fingers crossed)
-Hoss
