Skip to content

Comments

Setting @Ignore to fully ignore the string#194

Open
ollietb wants to merge 7 commits intoschmittjoh:masterfrom
ollietb:ignore
Open

Setting @Ignore to fully ignore the string#194
ollietb wants to merge 7 commits intoschmittjoh:masterfrom
ollietb:ignore

Conversation

@ollietb
Copy link

@ollietb ollietb commented Apr 14, 2014

I think the default behaviour for @ignore is misleading. It sounds like it should ignore the whole string, not just the error message. This PR adds the ability to @ignore a string in a Controller or FormType class. I don't currently see a way of implementing for twig yet.

Fixes #162 and #20

@hacfi
Copy link

hacfi commented Jun 21, 2014

👍

@ollietb Could you retrigger travis?

@ollietb
Copy link
Author

ollietb commented Jul 11, 2014

@hacfi I think I need to be the owner to retrigger travis.

@hacfi
Copy link

hacfi commented Jul 11, 2014

@ollietb No..just add a new commit. For example remove a blank line in the usage.rst file.

@Aaike
Copy link

Aaike commented Nov 5, 2014

what happened to this ? i think this is a good integration when will it be merged with the master branch ?

@hacfi
Copy link

hacfi commented Nov 5, 2014

politely ping @schmittjoh :)

@Nyholm
Copy link
Collaborator

Nyholm commented Mar 24, 2016

Cool @ollietb. Thank you for this PR.

Would you mind rebasing this on master?

@ollietb
Copy link
Author

ollietb commented Jul 7, 2016

@Nyholm is this still needed?

@Nyholm
Copy link
Collaborator

Nyholm commented Jul 7, 2016

I politely ask you the same question. =)

The two referenced tickets are closed. I was not familiar with the code base at the time you first made this PR.

What I really want to know is if you have time and interest if updating the PR and push it through a review process? If not we can close this for now until someone picks ut up.

@ollietb
Copy link
Author

ollietb commented Jul 14, 2016

@Nyholm Yep I can do it, I was just making sure that this hadn't been done by someone else in another PR

@ollietb
Copy link
Author

ollietb commented Jul 22, 2016

@Nyholm I've updated the PR

@Nyholm
Copy link
Collaborator

Nyholm commented Aug 4, 2016

When testing this with the following code in a controller:

        $form = $this->createFormBuilder();
        $form->add('status','choice', [
            'choices'   => [
                "1" => 'Yes',
                "0" => 'No'
            ],
            'required' => false,
        ]);

I get following result:

      <trans-unit id="356a192b7913b04c54574d18c28d46e6395428ab" resname="1">
        <source>1</source>
        <target state="new">1</target>
        <jms:reference-file line="26">/Users/tobias/Workspace/PHPStorm/SymfonyBundle/app/../src/AppBundle/Controller/DefaultController.php</jms:reference-file>
      </trans-unit>

With the current dev-master I get

      <trans-unit id="b6589fc6ab0dc82cf12099d1c2d40ab994e8410c" resname="0">
        <source>0</source>
        <target state="new">0</target>
        <jms:reference-file line="27">/Users/tobias/Workspace/PHPStorm/SymfonyBundle/app/../src/AppBundle/Controller/DefaultController.php</jms:reference-file>
      </trans-unit>
      <trans-unit id="356a192b7913b04c54574d18c28d46e6395428ab" resname="1">
        <source>1</source>
        <target state="new">1</target>
        <jms:reference-file line="26">/Users/tobias/Workspace/PHPStorm/SymfonyBundle/app/../src/AppBundle/Controller/DefaultController.php</jms:reference-file>
      </trans-unit>

Without any ignore this is not fetching all my translations.

$this->assertEquals($expected, $this->extract('MyAuthException.php'));
$extracted = $this->extract('MyAuthException.php');

$this->assertFalse($extracted->getDomains()['authentication']->has('security.authentication_error.ignored'));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not supported in PHP5.3

@ollietb
Copy link
Author

ollietb commented Aug 17, 2016

@Nyholm is there a test for that controller code? I can't seem to see it in the tests?

@peschee
Copy link

peschee commented Sep 16, 2016

Any updates on this or has it been abandoned @Nyholm @ollietb

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.

5 participants