-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[RFC] new interface for searching #16
Comments
@jippi since there is such an overwhelming interest from other people here is my implementation. Please let me know if this will work for you and I'll add it to my branch. /**
* Gets all filters in a given collection.
*
* @param string $collection Name of the filter collection.
* @return array Array of filter instances.
*/
public function getFilters($collection = 'default')
{
return $this->_filters[$collection];
}
/**
* Sets or gets the filter collection name.
*
* @param string $name Name of the active filter collection to set.
* @return mixed Returns $this or the name of the active collection if no $name was provided.
*/
public function collection($name = null) {
if ($name === null) {
return $this->_collection;
}
if (!isset($this->_filters[$name])) {
$this->_filters[$name] = [];
}
$this->_collection = $name;
return $this;
}
/**
* Adds a new filter to the active collection.
*
* @param string $name
* @param string $filter
* @param array $options
* @return $this
*/
public function add($name, $filter, array $options = [])
{
$this->_filters[$this->_collection][$name] = $this->_loadFilter($filter, $options);
return $this;
}
/**
* Loads a search filter instance.
*
* @param string $name Name of the filter class to load.
* @param array $options Filter options.
* @return \Search\Search\Type\Base
* @throws \InvalidArgumentException When no filter was found.
*/
public function _loadFilter($name, array $options = [])
{
list($plugin, $name) = pluginSplit($name);
if (!empty($plugin)) {
$className = '\\' . $plugin . '\Search\Type\\' . $name;
if (class_exists($className)) {
return new $className($name, $options, $this);
}
}
if (isset($config['typeClasses'][$name])) {
return new $config['typeClasses'][$name]($name, $options, $this);
}
if (class_exists('\FOC\Search\Search\Type\\' . $name)) {
$className = '\FOC\Search\Search\Type\\' . $name;
return new $className($name, $options, $this);
}
if (class_exists('\App\Search\Type\\' . $name)) {
$className = '\App\Search\Type\\' . $name;
return new $className($name, $options, $this);
}
throw new \InvalidArgumentException(sprintf('Can\'t find filter class %s!', $name));
} |
@jippi I've implented this proposal in my fork of the plugin, feel free to take it from there or just close this ticket. |
Why a trait and not a behaviour? It makes more sense to be that if it's dealing with the table classes it should be a behaviour. Using a trait feels like a hack to me, like it's sidestepping the framework. |
@davidyell where do you see a trait? |
Sorry, I should have tagged Jippi,
Oh, but it looks like it's quoting you. So my question was why use a trait to return the manager rather than a behaviour. |
@jippi you can close this. |
See #46 |
Closing as PR is open. |
(Originally posted by @burzum on the wiki)
Instead of relying on the presence of
searchConfiguration()
in the table class I think a trait that adds afilters()
method that returns a manager instance would be better. The interface could be similar to validation:The text was updated successfully, but these errors were encountered: