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

EditorBuilder registers deprecated PhpSearchStrategy #120

Open
davidgorges opened this issue Apr 4, 2015 · 8 comments
Open

EditorBuilder registers deprecated PhpSearchStrategy #120

davidgorges opened this issue Apr 4, 2015 · 8 comments
Assignees
Labels

Comments

@davidgorges
Copy link

The triggered \E_USER_DEPRECATED errors cause phpunit to fail.

If anyone else runs into that:
\PHPUnit_Framework_Error_Deprecated::$enabled = FALSE;
disables the test fails.

@gnugat
Copy link
Owner

gnugat commented Apr 5, 2015

Hi @davidgorges thanks for your report. I'll remove PhpSearchStrategy from EditorBuilder to avoid this notice.

@gnugat gnugat added the bug label Apr 5, 2015
@gnugat gnugat self-assigned this Apr 5, 2015
@wouterj
Copy link
Contributor

wouterj commented Apr 5, 2015

@gnugat it can't be removed, as it's there for BC reasons. I think there are 2 options:

  1. Instead of adding it as first search strategy, add it as last, so with normal usage, the notice won't be triggered (initializing the class doesn't trigger the deprecation notice)
  2. Remove the trigger from PhpSearchStrategy#supports(). Again, this would avoid getting the error, unless someone uses the PhpSearchStrategy explicitely. In that case, it should get this notice.

@gnugat
Copy link
Owner

gnugat commented Apr 5, 2015

@wouterj good point, I'll try your first advice, if it doesn't do the trick I'll check the second :)

@gnugat gnugat mentioned this issue Apr 5, 2015
@gnugat
Copy link
Owner

gnugat commented Apr 5, 2015

@davidgorges could you provide us with a small piece of code to reproduce the issue? I'd like to add a test for it.

@wouterj
Copy link
Contributor

wouterj commented Apr 5, 2015

@gnugat I think this will do (didn't test it though):

$editor = EditorFactory::createEditor();
$editor->findBelow('something');

@gnugat
Copy link
Owner

gnugat commented Apr 5, 2015

That's weird, example is full of tests that use EditorFactory::createEditor() + $editor#jumpBelow() and we never had a failure caused by PHP notice...

@wouterj
Copy link
Contributor

wouterj commented Apr 5, 2015

@gnugat try with a line number search pattern. As that's the only strategy registered after the PHP search strategy (I forget about the priorty argument my previous comments). https://github.com/gnugat/redaktilo/blob/master/src/Gnugat/Redaktilo/Service/EditorBuilder.php#L86-L89

With all other patterns, there is a matching strategy stopping the iteration.

gnugat pushed a commit that referenced this issue Apr 6, 2015
@gnugat
Copy link
Owner

gnugat commented Apr 6, 2015

@wouterj good point! LineNumberSearchStrategy is also deprecated, so I've added a test checking the notice (should be about LineNumberSearchStrategy and not PhpSearchStrategy).

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

No branches or pull requests

3 participants