Skip to content

lib_phpQuery support PHP 8#3186

Merged
Alkarex merged 7 commits intoFreshRSS:masterfrom
prashanttholia:support-php-8
Oct 3, 2020
Merged

lib_phpQuery support PHP 8#3186
Alkarex merged 7 commits intoFreshRSS:masterfrom
prashanttholia:support-php-8

Conversation

@prashanttholia
Copy link
Contributor

Project updates to support PHP 8.

Closes #3082

Changes proposed in this pull request:

How to test the feature manually:

Pull request checklist:

  • clear commit messages
  • code manually tested
  • unit tests written (optional if too hard)
  • documentation updated

Replaced create_function calls with anonymous functions in lib/lib_phpQuery.php

Ref Issue: #3082
@Alkarex Alkarex added this to the 1.18.0 milestone Sep 24, 2020
@Alkarex
Copy link
Member

Alkarex commented Sep 24, 2020

Thanks @prashanttholia , that looks good. I will have a look shortly.
Did you already test whether your changes are working?

@prashanttholia
Copy link
Contributor Author

Hi @Alkarex, I had tested separately only the update I made in CallbackBody class, but did not test the other updates. I'll test them too and post about it here.

@Alkarex Alkarex changed the title Support PHP 8 lib_phpQuery support PHP 8 Oct 3, 2020
Fix: Fatal error: Array and string offset access syntax with curly
braces is no longer supported in /FreshRSS/lib/lib_phpQuery.php on line
2174, etc.
@Alkarex
Copy link
Member

Alkarex commented Oct 3, 2020

Let's continue testing in the main branch.
@prashanttholia Please add a line for you in https://github.com/FreshRSS/FreshRSS/blob/master/CREDITS.md

@Alkarex Alkarex merged commit 83166a9 into FreshRSS:master Oct 3, 2020
@prashanttholia
Copy link
Contributor Author

@Alkarex, I have been working on points to integrate PHP 8 changes. I understand that I have not been as prompt, but I would request you to allow me to do it. I request you to let me know whether shall I continue to use this PR to commit my updates?

@Alkarex
Copy link
Member

Alkarex commented Oct 3, 2020

@prashanttholia You are very welcome. Please open a new PR. It was fine with that PR focussing on a single library

@prashanttholia
Copy link
Contributor Author

@Alkarex, okay, thanks a lot. One more point I wanted to mention was that I had tested the changes made in the library. The changes were made at 3 places in the file -

  1. CallbackBodyClass
  2. phpQueryObject::pseudoClasses method
  3. phpQuery::markupToPHP method

I wanted to mention few points about these.
For 1, I could not find the class being used anywhere in the project. So I have tested the new callback function in the class using sample functions in a separate script.
For 2, I have tested all cases where the changes were made using the FreshRSS management interface. All changes produced same results as the original script for variety of selector values for each case. I would, though, would want to bring into your notice that I found that the selector values "input" & ":enabled" did not produce valid results in both the original and the updated scripts.
For 3, I could not find a source in the project from where this was directly/indirectly being called and I have not tested it yet. I believe the implementation is correct though. I shall it test it any way and post my update here.

Thanks.

@Alkarex
Copy link
Member

Alkarex commented Oct 3, 2020

Thanks for the detailed explanation @prashanttholia , and that looks sufficient for now 👍

@prashanttholia
Copy link
Contributor Author

Okay, thanks.

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.

Support PHP 8

3 participants