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

Compatibility with PHP 7.4, PHP 8.0 and migrate to Laminas Escaper #1946

Merged
merged 23 commits into from
Dec 29, 2020
Merged

Compatibility with PHP 7.4, PHP 8.0 and migrate to Laminas Escaper #1946

merged 23 commits into from
Dec 29, 2020

Conversation

liborm85
Copy link
Contributor

@liborm85 liborm85 commented Oct 17, 2020

Description

  • compatibility with PHP 7.4
  • compatibility with PHP 8.0 (for full support require update in 3rd libraries)
  • travis-ci: fixed execution on PHP 5.x
  • fixed scrutinizer, allow zip extension
  • migrate from abandoned Zend\Escaper to Laminas Escaper

For full compatibility with php 8.0 require PR PHPOffice/Common#34

Fixes #1944
Fixes #1874
Fixes #1797

Checklist:

  • I have run composer run-script check --timeout=0 and no errors were reported
  • The new code is covered by unit tests (check build/coverage for coverage report)
  • I have updated the documentation to describe the changes

@liborm85 liborm85 changed the title [WIP] PHP 8.0 in Travis [WIP] Compatibility with PHP 7.4 and PHP 8.0 Oct 18, 2020
@liborm85 liborm85 changed the title [WIP] Compatibility with PHP 7.4 and PHP 8.0 [WIP] Compatibility with PHP 7.4, PHP 8.0 and migrate to Laminas Escaper Oct 18, 2020
@liborm85 liborm85 changed the title [WIP] Compatibility with PHP 7.4, PHP 8.0 and migrate to Laminas Escaper [WIP] Compatibility with PHP 7.4, PHP 8.0 and migrate to Laminas Escaper Oct 18, 2020
@liborm85 liborm85 mentioned this pull request Oct 18, 2020
@@ -454,7 +454,7 @@ private function setSourceType()
} else {
$this->sourceType = self::SOURCE_GD;
}
} elseif (@file_exists($this->source)) {
} elseif ((strpos($this->source, chr(0)) === false) && @file_exists($this->source)) {

Choose a reason for hiding this comment

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

We could use PHP8-functions here, with a polyfill from Symfony.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see any new feature in php8 or symfony polyfills to solve this problem.

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

str_contains doesn't solve much... Symfony polyfills require PHP 7, not compatible with PHP 5.x https://github.com/symfony/polyfill/blob/master/src/Php80/composer.json#L23

Copy link

@stephanvierkant stephanvierkant Oct 19, 2020

Choose a reason for hiding this comment

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

Is that a serious problem? We should drop support for PHP 5 ASAP: #1948

And no, it ‘str_contains‘ doesn't solve a problem, but I think it's nicer. It improves readability for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is problem, because this PR is about compatibility with PHP 8 and PHP 7.4, not about drop support old PHP versions.

Choose a reason for hiding this comment

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

That's true. I opened an issue, I'd like to hear your opinion :)

@liborm85 liborm85 changed the title [WIP] Compatibility with PHP 7.4, PHP 8.0 and migrate to Laminas Escaper Compatibility with PHP 7.4, PHP 8.0 and migrate to Laminas Escaper Oct 20, 2020
@liborm85 liborm85 marked this pull request as ready for review October 20, 2020 05:23
@liborm85
Copy link
Contributor Author

@troosan Can you review it?

@michaljusiega
Copy link

Let's release this changes please ;)

@liborm85 liborm85 mentioned this pull request Nov 27, 2020
3 tasks
@dsuurlant
Copy link

Looking forward to this PR being merged! Is there anything I could do to contribute?

@stephanvierkant
Copy link

@troosan doesn't seem to be active here. How can we contact him? Are there any other maintainers?

@liborm85 liborm85 mentioned this pull request Dec 8, 2020
@troosan troosan added this to the v0.18.0 milestone Dec 29, 2020
@troosan troosan merged commit 7a97d24 into PHPOffice:develop Dec 29, 2020
@troosan
Copy link
Contributor

troosan commented Dec 29, 2020

@stephanvierkant Thanks for this nice work. I've been quite absent for quite some time indeed.
I'll take some time to merge what can be merged and release a new version.

@stephanvierkant
Copy link

Hi @troosan, welcome back! And you're welcome, but credits to @liborm85 for this PR.

@liborm85 liborm85 deleted the php80 branch January 1, 2021 14:51
@troosan
Copy link
Contributor

troosan commented Feb 6, 2021

indeed, will definitely mention it in the release notes

mussbach added a commit to mussbach/PHPWord that referenced this pull request Mar 4, 2021
…/laminas-zendframework-bridge

This commit fixes the BC break introduced by PHPOffice#1946 where it was stated
that phpword replaces package laminas/laminas-zendframework-bridge which
is not the case. Package laminas/laminas-zendframework-bridge only was
replaced from phpword.
PowerKiKi added a commit that referenced this pull request Mar 8, 2021
Fix BC break in #1946. This package does not replace laminas/laminas-zendframework-bridge
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants