I am not in favour of this sort of hidden secret pull request.

Hey Brian,

Thanks for giving feedback so quickly, but could you please clarify? :) What do you mean by hidden secret and what can I do to improve it?

its a hidden secret because unless someone reads the code they wouldnt know it was possible to include images in the search results

Not to mention it's also inconsistent as other data types have some form of image data available to them.

Thanks for the feedback guys, I guess it's better if I close the pull request then!

Please consider reopening this PR.
It's true that other data types don't have images but this is not reason to not use them for content articles.

Besides search results template it's super easy to check if result has an image isset($this->result->images)

You don't need to modify the smart search content plugin to achieve this. You can create your own plugin that will extend the core plugin to add whatever extra fields you want to the search index. That's what the FinderIndexerHelper::getContentExtras($item); line is for.

@chrisdavenport

Yes, I'm aware of that, but images and URLs are native Joomla types - not extra fields added by 3p plugin.
The thing is that these fields are not picked up when building a query.

It doesn't make sense to build a plugin with onPrepareFinderContent method (which is called by mentioned FinderIndexerHelper::getContentExtras($item)) because these fields are just not available. I'd have to create a new database query for same table just to pick them up for each item.

Anyway these fields should not be decoded as views expect strings in json format and shouldn't be searchable, just available as item property for articles, categories, tags and contacts.

Why the extension is indexing things like list_price, sale_price, comments but not native fields?

My current workaround is to monkey patch the PlgFinderContent plugin and add add images and urls fields to the select query like in the PR.

JLoader::register('PlgFinderContent', JPATH_ROOT . '/plugins/finder/content/content.php');

class PlgFinderPcz_content extends PlgFinderContent
{
    protected function getListQuery($query = null)
    {
        return parent::getListQuery($query)
            ->select('a.images AS images')
            ->select('a.urls AS urls');
    }
}

but images and URLs are native Joomla types - not extra fields added by 3p plugin.
The thing is that these fields are not picked up when building a query.

You could probably make the same argument for other fields in the content table too. But the extra data takes more space in the payload, hence more memory, while most people won't need it. Extending is easy and there are severals ways it can be done depending on your requirements.

It doesn't make sense to build a plugin with onPrepareFinderContent method (which is called by mentioned FinderIndexerHelper::getContentExtras($item)) because these fields are just not available. I'd have to create a new database query for same table just to pick them up for each item.

Sure, it's one extra query, but only during indexing. If you're worried about that then just use your "monkey patch" approach; nothing wrong with that (although I'd normallly argue against one extension modifying another's database queries). Alternatively, you have the article id, so you could do the extra query in the view (purists should avert their gaze) and you can do that as one SELECT per page of results. Choose the path that is right for you.

Anyway these fields should not be decoded as views expect strings in json format and shouldn't be searchable, just available as item property for articles, categories, tags and contacts.

You have complete control over what is indexed and how. Adding a property into $item will merely add it to the payload. You must call addInstruction to add it to the (searchable) index and/or addTaxonomy to add it to a content map.

Why the extension is indexing things like list_price, sale_price, comments but not native fields?

The list_price and sale_price fields (which really should be renamed, but that would be a b/c break) are required because we need to have at least one numerically sortable field available. It's inefficient to try to sort using fields that only exist in the payload blob. I can't see a field called "comments" so I can't comment on that one :p

My current workaround is to monkey patch the PlgFinderContent plugin and add add images and urls fields to the select query like in the PR.

I wouldn't call it a workaround; I think that's a perfectly acceptable way of extending the core code.

Thank you for the insight.
Now I think that images should be picked up in view by triggering new event - perhaps onFinderContentPrepare.
Every finder plugin could decide which data (besides those indexed) should be set to be used by view and how to format it.