MrDys / blacklight

Blacklight Plugin
http://projectblacklight.org/
Other
1 stars 1 forks source link

ApplicationHelper#render_document_show_field_value should enter each element of an Array on a separate line #448

Closed MrDys closed 12 years ago

MrDys commented 12 years ago

CODEBASE-266: In ApplicationHelper#render_document_show_field_value it looks as if the desired output is for a field value that is an Array to be rendered with each element on a separate line. The problem is that SolrDocument#get from rsolr-ext has a default separator of a comma and space:

https://github.com/mwmitchell/rsolr-ext/blob/51f212e62440ff8c96559b64fb99bb885a0df58b/lib/rsolr-ext/doc.rb

In order to get the field value back as an Array you must pass in nil as the separator. So render_document_show_field_value ought to look like this:

def render_document_show_field_value args value = args[:value] value ||= args[:document].get(args[:field], :sep => nil) if args[:document] and args[:field] return value.map { |v| html_escape v }.join "
" if value.is_a? Array html_escape value end

MrDys commented 12 years ago

Original reporter: jronallo

MrDys commented 12 years ago

cbeer: It's not obvious to me what the desired behavior actually is.. part of resolving this bug probably should include reconciling the index and show value displays.

MrDys commented 12 years ago

jronallo: It seemed to me that the expected (maybe not desired) behavior here is for any field value that is an Array to display on separate lines. Otherwise there's no reason to have the check for an Array. But in this case the current use of SolrDocument.get will never return an Array and will only return a comma separated list. This means the current code will never give us a chance to do anything different with the display of array values ourselves unless we explicitly pass in :sep => nil.

I think getting an Array back from SolrDocument#get is just a first step until we figure out how we want to do something else. For instance can we figure out the best way to make sure that any facet values get turned into search links? Is that possible now? Can we have some way to set the separator differently for each field? It seems that with small blocks of text comma or pipe delimited may be what some implementers want while with longer blocks of text for each value in an array, it'd be better to have a line break or a paragraph as the way to separate values within the same field.

But without getting back an Array from SolrDocument#get we're leaving it up to rsolr-ext to make the decision to use commas as the separator.

MrDys commented 12 years ago

cbeer: i'm going to split out the value mapping and the field value separator into two separate methods.

maybe overkill, but i'd personally like to wrap multiple field values in a

MrDys commented 12 years ago

cbeer: Fixed in https://github.com/projectblacklight/blacklight/commit/9fb107db910d934b6484639df88b23009647e269