Skip to content
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

Fix index wide finder config check #62

Merged
merged 1 commit into from Jan 6, 2012
Merged

Conversation

merk
Copy link
Member

@merk merk commented Jan 6, 2012

Turns out isset() doesnt work as expected for null values. Not sure how I missed this in my initial testing. :S

Fixes #61

@jmikola
Copy link
Member

jmikola commented Jan 6, 2012

Using array_key_exists() means it would be impossible to override the finder value in a configuration (consider multiple config blocks being merged). What about:

    /**
     * Adds the configuration for the "indexes" key
     */
    private function addIndexesSection(ArrayNodeDefinition $rootNode)
    {
        $rootNode
            ->fixXmlConfig('index')
            ->children()
                ->arrayNode('indexes')
                    ->useAttributeAsKey('name')
                    ->prototype('array')
                        ->performNoDeepMerging()
                        ->children()
                            ->scalarNode('client')->end()
                            ->scalarNode('finder')
                                ->treatNullLike(true)
                                ->defaultFalse()
                            ->end()

Using YAML as an example, that would allow users to enable a finder with true and ~, but disable it with false. And in the extension, we could replace the isset() with a simple variable check since the key should always exist.

Fixes index wide finder configuration. Fixes FriendsOfSymfony#61
@merk
Copy link
Member Author

merk commented Jan 6, 2012

Cant say I knew about treatNullLike. Thanks. Changed.

@jmikola
Copy link
Member

jmikola commented Jan 6, 2012

Thanks!

jmikola added a commit that referenced this pull request Jan 6, 2012
Fix index wide finder config check
@jmikola jmikola merged commit 2193976 into FriendsOfSymfony:master Jan 6, 2012
jmikola added a commit that referenced this pull request Jan 12, 2012
Fix index wide finder config check
@merk merk deleted the patch-1 branch July 4, 2014 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Index wide finder is not registered
2 participants