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

Allow extending of the Extractor for custom form options #377

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ollietb
Copy link

@ollietb ollietb commented Jul 26, 2016

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License Apache2

Description

My forms have some extra translatable options in them (like "help_text") which are provided by MopaBootstrapBundle. I need to customise the FormExtractor::enterNode() method but because of the private methods it makes this impossible.

Todos

  • [ na ] Tests
  • [ na ] Documentation
  • [ done ] Changelog

@Nyholm
Copy link
Collaborator

Nyholm commented Aug 4, 2016

This PR is mixed with code style changes. One should avoid that to make it easier to review.

Im not too sure I like making private methods public. Tell me more about your use case. Can I see a copy of your class that extends the FormExtractor? It must be a more SOLID way of doing this.

@ollietb
Copy link
Author

ollietb commented Aug 17, 2016

@Nyholm this is my class that extends the FormExtractor

class MopaBootstrapExtractor extends FormExtractor
{

    /**
     * @param Node $node
     * @return null
     */
    public function enterNode(Node $node)
    {
        if ($node instanceof Node\Stmt\Class_) {
            $this->defaultDomain = null;
            $this->defaultDomainMessages = array();
        }

        if ($node instanceof Node\Expr\MethodCall) {
            if (!is_string($node->name)) {
                return;
            }

            $name = strtolower($node->name);
            if ('setdefaults' === $name || 'replacedefaults' === $name) {
                $this->parseDefaultsCall($node);
                return;
            }
        }

        if ($node instanceof Node\Expr\Array_) {
            // first check if a translation_domain is set for this field
            $domain = $this->getDomain($node);

            // look for options containing a message
            foreach ($node->items as $item) {
                if (!$item->key instanceof Node\Scalar\String_) {
                    continue;
                }

                switch ($item->key->value) {
                    case 'label':
                    // this extra info is used in MopaBootstrapBundle
                    case 'help':
                    case 'help_text':
                    case 'help_block':
                    case 'help_inline':
                        $this->parseItem($item, $domain);
                        break;
                    case 'invalid_message':
                        $this->parseItem($item, 'validators');
                        break;
                    case 'placeholder':
                    case 'empty_value':
                        if ($this->parseEmptyValueNode($item, $domain)) {
                            continue 2;
                        }
                        $this->parseItem($item, $domain);
                        break;
                    case 'choices':
                        if ($this->parseChoiceNode($item, $node, $domain)) {
                            continue 2;
                        }
                        $this->parseItem($item, $domain);
                        break;
                    case 'attr':
                        if ($this->parseAttrNode($item, $domain)) {
                            continue 2;
                        }
                        $this->parseItem($item, $domain);
                        break;

                }
            }
        }
    }
}

@Nyholm
Copy link
Collaborator

Nyholm commented Aug 17, 2016

So what you really want to do here is to add more cases to the switch statement?

@ollietb
Copy link
Author

ollietb commented Aug 17, 2016

@Nyholm in this case yes, but it's possible that there could be a requirement for parsing more custom formats within complex forms, like nested arrays with certain keys that need translating.

@VSilantyev
Copy link

I have the same scenario, added "case 'help'" to my branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants