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 \SensioLabs\AnsiConverter\AnsiToHtmlConverter to be injected #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rquadling
Copy link

As a consequence, the need to be able to set a theme at run time, rather
than at compile time is necessary.

Also added PHPUnit require-dev support and added tests to show the theme
can be injected and overridden.

composer.json Outdated
@@ -12,6 +12,9 @@
"require": {
"php": ">=5.3.0"
},
"require-dev": {
"phpunit/phpunit": "^4"
Copy link
Member

Choose a reason for hiding this comment

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

That should be removed, I never require phpunit in my composer.json. We can use symfony/phpunit-bridge instead.

Copy link
Author

Choose a reason for hiding this comment

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

It is require-dev, so isn't part of the deployment requirement. The use case for its presence is for a standalone package. I've not used symfony/phpunit-bridge, so can you show me what this should be please?

Copy link
Author

Choose a reason for hiding this comment

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

Hi.

I don't know what what should be here if you want to run phpunit in travis.
Having also dropped support for PHP < 7, there's a minimum version of PHPUnit of 6 (it seems).

If you can "fix" this so that the tests are run in travis, then I'd be happy with that.

@rquadling
Copy link
Author

At this stage, I'd be dropping PHP 5.3. It IS end of life and I wonder if the tests ever passed.

If a new major release was created for this PR, then that would be the time to drop 5.3 (and maybe 5.4 also).

@rquadling
Copy link
Author

@fabpot Any idea on what I can do for the failing unit tests on PHP 5.3? Also, can you give me some more info on the PHPUnit issue in the composer.json. Happy to fix things.

@rquadling
Copy link
Author

https://3v4l.org/oqB5b

Testing the regex against one of the unit tests ansi encoded strings.

Output for 5.4.0 - 5.6.30, hhvm-3.10.1 - 3.19.0, 7.0.0 - 7.1.4 is all OK

Output for 4.3.0 - 5.3.29 fails

In further examination, this seems to be due to PHP <5.4 not understanding escape in a string.

At this stage, I'd say dropping PHP < 5.4 is a reasonable proposition.

As a consequence, the need to be able to set a theme at run time, rather
than at compile time is necessary.

Also added PHPUnit require-dev support and added tests to show the theme
can be injected and overridden.
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.

2 participants