-
Notifications
You must be signed in to change notification settings - Fork 191
2.0 (WIP) #111
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
2.0 (WIP) #111
Changes from 10 commits
bc9c824
2e811ee
d914bfc
8834d44
6588053
4c91066
8ab9519
acff817
7fd7152
4e7c36c
8e20106
c6359dc
ea182c6
e3b7322
d1452b4
d48d4f3
b383440
fa040ce
06dd977
10ec2eb
e1d0d5e
5e3d8d4
83dd16b
22e6c63
ec045ca
e6d3d62
560c9f7
b1c0d16
a2d94cc
174bb9e
b09df2a
21d03f6
2480865
e22d569
de745ff
2851b4a
b016b8d
f622480
af10a75
c82c919
4b9c31b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,2 @@ | ||
| /vendor | ||
| composer.lock | ||
| Gemfile.lock |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,20 +1,24 @@ | ||
| language: php | ||
|
|
||
| php: | ||
| - 5.3 | ||
| - 5.4 | ||
| - 5.5 | ||
| - 5.6 | ||
| - 7.0 | ||
| - hhvm | ||
|
|
||
| matrix: | ||
| include: | ||
| - php: 5.4 | ||
| - php: 5.5 | ||
| - php: 5.6 | ||
| - php: 7 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 7.0 would be better
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in 174bb9e |
||
| - php: nightly | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nightly's not really needed
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in 174bb9e |
||
| - php: hhvm | ||
| allow_failures: | ||
| - php: 7.0 | ||
| - php: nightly | ||
| - php: hhvm | ||
|
|
||
| before_script: | ||
| - travis_retry composer self-update | ||
| - travis_retry composer install --no-interaction --prefer-source --dev | ||
| - composer self-update | ||
| - composer install --prefer-source --no-interaction --no-scripts | ||
|
|
||
| script: | ||
| - vendor/bin/phpunit --verbose --coverage-text | ||
| - vendor/bin/phpunit --verbose --coverage-clover=coverage.clover | ||
|
|
||
| after_success: | ||
| - if [[ "$TRAVIS_PHP_VERSION" != "hhvm" ]] && [[ "$TRAVIS_PHP_VERSION" != "nightly" ]]; then wget https://scrutinizer-ci.com/ocular.phar; fi | ||
| - if [[ "$TRAVIS_PHP_VERSION" != "hhvm" ]] && [[ "$TRAVIS_PHP_VERSION" != "nightly" ]]; then php ocular.phar code-coverage:upload --format=php-clover coverage.clover; fi | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,31 +1,25 @@ | ||
| { | ||
| "name": "tijsverkoyen/css-to-inline-styles", | ||
| "type": "library", | ||
| "name": "tijsverkoyen/css-to-inline-styles", | ||
| "type": "library", | ||
| "description": "CssToInlineStyles is a class that enables you to convert HTML-pages/files into HTML-pages/files with inline styles. This is very useful when you're sending emails.", | ||
| "homepage": "https://github.com/tijsverkoyen/CssToInlineStyles", | ||
| "license": "BSD", | ||
| "authors": [ | ||
| "homepage": "https://github.com/tijsverkoyen/CssToInlineStyles", | ||
| "license": "BSD-3-Clause", | ||
| "authors": [ | ||
| { | ||
| "name": "Tijs Verkoyen", | ||
| "name": "Tijs Verkoyen", | ||
| "email": "[email protected]", | ||
| "role": "Developer" | ||
| "role": "Developer" | ||
| } | ||
| ], | ||
| "require": { | ||
| "php": ">=5.3.0", | ||
| "symfony/css-selector": "~2.1" | ||
| "require": { | ||
| "symfony/css-selector": "^2.7" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should keep the PHP requirement to specify the min version
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed it, because in the real world there are a lot of outdated servers. I won't support issues with those older version, but I don't want to force them also.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👎
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should defintely keep it
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do you guys handle outdated hosting? Do you force everyone to upgrade their servers? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. People will never update hosting if libraries keep supporting them. I'm also 👍 for forcing a minimum php version
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Laravel, Symfony, Twig, Guzzle, Drupal 8 etc all require 5.5. PHP 5.5 is already security updates only; http://php.net/supported-versions.php If people need to use old hosting, they can still use the current version.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the 1.x branch supports Symfony 3 now. Your 2.0 version should support it too
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in b016b8d |
||
| }, | ||
| "require-dev": { | ||
| "phpunit/phpunit": "~4.0" | ||
| "phpunit/phpunit": "~4.8" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should also allow phpunit 5, not only 4.8 (4.8 does not officially support PHP 7, even if it mostly works)
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in 2851b4a |
||
| }, | ||
| "autoload": { | ||
| "autoload": { | ||
| "psr-4": { | ||
| "TijsVerkoyen\\CssToInlineStyles\\": "src" | ||
| } | ||
| }, | ||
| "extra": { | ||
| "branch-alias": { | ||
| "dev-master": "1.5.x-dev" | ||
| } | ||
| } | ||
| } | ||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,7 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
|
|
||
| <phpunit backupGlobals="false" | ||
| backupStaticAttributes="false" | ||
| bootstrap="vendor/autoload.php" | ||
| cacheTokens="true" | ||
| colors="true" | ||
| convertErrorsToExceptions="true" | ||
| convertNoticesToExceptions="true" | ||
|
|
@@ -19,9 +17,10 @@ | |
| </testsuites> | ||
|
|
||
| <filter> | ||
| <whitelist addUncoveredFilesFromWhitelist="false"> | ||
| <whitelist addUncoveredFilesFromWhitelist="true"> | ||
| <directory suffix=".php">src</directory> | ||
| <exclude> | ||
| <directory suffix=".php">tests</directory> | ||
| <directory suffix=".php">vendor</directory> | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this exclude rules are useless, as
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in 174bb9e |
||
| </exclude> | ||
| </whitelist> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| <?php | ||
|
|
||
| namespace TijsVerkoyen\CssToInlineStyles\Css; | ||
|
|
||
| use TijsVerkoyen\CssToInlineStyles\Css\Rule\Processor as RuleProcessor; | ||
|
|
||
| class Processor | ||
| { | ||
| /** | ||
| * Get the rules from a given CSS-string | ||
| * | ||
| * @param string $css | ||
| * @return array | ||
| */ | ||
| public function getRules($css) | ||
| { | ||
| $css = $this->doCleanup($css); | ||
| $rulesProcessor = new RuleProcessor(); | ||
| $rules = $rulesProcessor->splitIntoSeparateRules($css); | ||
|
|
||
| return $rulesProcessor->convertArrayToObjects($rules); | ||
| } | ||
|
|
||
| /** | ||
| * @param $css | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. missing type here
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in e3b7322 |
||
| * @return mixed|string | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the return value is always a string
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
| */ | ||
| protected function doCleanup($css) | ||
| { | ||
| // remove media queries | ||
| $css = preg_replace('/@media [^{]*{([^{}]|{[^{}]*})*}/', '', $css); | ||
|
|
||
| $css = str_replace(array("\r", "\n"), '', $css); | ||
| $css = str_replace(array("\t"), ' ', $css); | ||
| $css = str_replace('"', '\'', $css); | ||
| $css = preg_replace('|/\*.*?\*/|', '', $css); | ||
| $css = preg_replace('/\s\s+/', ' ', $css); | ||
| $css = trim($css); | ||
|
|
||
| return $css; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| <?php | ||
|
|
||
| namespace TijsVerkoyen\CssToInlineStyles\Css\Property; | ||
|
|
||
| class Processor | ||
| { | ||
| /** | ||
| * Split a string into seperate properties | ||
| * | ||
| * @param string $propertiesString | ||
| * @return array | ||
| */ | ||
| public function splitIntoSeparateProperties($propertiesString) | ||
| { | ||
| $propertiesString = $this->cleanup($propertiesString); | ||
|
|
||
| $properties = (array) explode(';', $propertiesString); | ||
| $keysToRemove = array(); | ||
| $numberOfProperties = count($properties); | ||
|
|
||
| for ($i = 0; $i < $numberOfProperties; $i++) { | ||
| $properties[$i] = trim($properties[$i]); | ||
|
|
||
| // if the new property begins with base64 it is part of the current property | ||
| if (isset($properties[$i + 1]) && strpos(trim($properties[$i + 1]), 'base64,') === 0) { | ||
| $properties[$i] .= ';' . trim($properties[$i + 1]); | ||
| $keysToRemove[] = $i + 1; | ||
| } | ||
| } | ||
|
|
||
| if (!empty($keysToRemove)) { | ||
| foreach ($keysToRemove as $key) { | ||
| unset($properties[$key]); | ||
| } | ||
| } | ||
|
|
||
| return array_values($properties); | ||
| } | ||
|
|
||
| /** | ||
| * @param $string | ||
| * @return mixed|string | ||
| */ | ||
| protected function cleanup($string) | ||
| { | ||
| $string = str_replace(array("\r", "\n"), '', $string); | ||
| $string = str_replace(array("\t"), ' ', $string); | ||
| $string = str_replace('"', '\'', $string); | ||
| $string = preg_replace('|/\*.*?\*/|', '', $string); | ||
| $string = preg_replace('/\s\s+/', ' ', $string); | ||
|
|
||
| $string = trim($string); | ||
| $string = rtrim($string, ';'); | ||
|
|
||
| return $string; | ||
| } | ||
|
|
||
| /** | ||
| * Convert a property-string into an object | ||
| * | ||
| * @param string $property | ||
| * @return Property|null | ||
| */ | ||
| public function convertToObject($property) | ||
| { | ||
| $chunks = (array) explode(':', $property, 2); | ||
|
|
||
| if (!isset($chunks[1]) || $chunks[1] == '') { | ||
| return null; | ||
| } | ||
|
|
||
| return new Property(trim($chunks[0]), trim($chunks[1])); | ||
| } | ||
|
|
||
| /** | ||
| * Convert an array of property-strings into objects | ||
| * | ||
| * @param array $properties | ||
| * @return array | ||
| */ | ||
| public function convertArrayToObjects(array $properties) | ||
| { | ||
| $objects = array(); | ||
|
|
||
| foreach ($properties as $property) { | ||
| $objects[] = $this->convertToObject($property); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't you skip it in case
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in d1452b4 |
||
| } | ||
|
|
||
| return $objects; | ||
| } | ||
|
|
||
| /** | ||
| * Build the property-string for multiple properties | ||
| * | ||
| * @param array $properties | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest documenting it as
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in e3b7322
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is not fixed here (you may have fixed a different place, but not this one)
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in e22d569 |
||
| * @return string | ||
| */ | ||
| public function buildPropertiesString(array $properties) | ||
| { | ||
| $chunks = array(); | ||
|
|
||
| foreach ($properties as $property) { | ||
| $chunks[] = $property->toString(); | ||
| } | ||
|
|
||
| return implode(' ', $chunks); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd recommend dropping php 5.4 support too tbh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe set the min version to 5.5.9?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
travis lets you test on 5.5.9 as well as 5.5.x
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 174bb9e