Skip to content

Replace lib_phpQuery by PhpGt/CssXPath#4261

Merged
Alkarex merged 5 commits intoFreshRSS:edgefrom
Alkarex:replace-lib_phpQuery
Mar 16, 2022
Merged

Replace lib_phpQuery by PhpGt/CssXPath#4261
Alkarex merged 5 commits intoFreshRSS:edgefrom
Alkarex:replace-lib_phpQuery

Conversation

@Alkarex
Copy link
Copy Markdown
Member

@Alkarex Alkarex commented Mar 5, 2022

@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented Mar 6, 2022

Ping @marienfressinaud and @aledeg : if you use full content retrieval for some of your feeds, it would be nice if you could give this a try

@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented Mar 6, 2022

@javerous You did a a great work regarding full content retrieval #2778 ; so if you are around, it would be nice if you could give this PR a try, as there are risks of regressions :-)
And if you have not seen it, you might be interested in this slightly related feature, which freshly landed in the edge branch: #4220

@math-GH
Copy link
Copy Markdown
Contributor

math-GH commented Mar 6, 2022

First glance it looks fine. Did not checked the code.

paths:
- .
excludePaths:
- lib/lib_phpQuery.php
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should CssXPath be excluded from phpstan scan?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Exclusion doesn't seem necessary if it's not causing issues, but potentially fair point from a handful of wasted cpu cycles.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

For now, I would rather keep it (phpstan uses some caching) to be informed in case of regression

@marienfressinaud
Copy link
Copy Markdown
Member

Ping @marienfressinaud and @aledeg : if you use full content retrieval for some of your feeds, it would be nice if you could give this a try

For the few I had, it seems to work well 👍 (I prefer your approach rather than previous lib_phpQuery btw :))

@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented Mar 16, 2022

For the few I had, it seems to work well 👍 (I prefer your approach rather than previous lib_phpQuery btw :))

Cool :-) Let's merge then

@Alkarex Alkarex merged commit ae54a59 into FreshRSS:edge Mar 16, 2022
@Alkarex Alkarex deleted the replace-lib_phpQuery branch March 16, 2022 14:10
@javerous
Copy link
Copy Markdown
Contributor

javerous commented Jun 1, 2022

@javerous You did a a great work regarding full content retrieval #2778 ; so if you are around, it would be nice if you could give this PR a try, as there are risks of regressions :-)
And if you have not seen it, you might be interested in this slightly related feature, which freshly landed in the edge branch: #4220

Hi @Alkarex - sorry for the late reply.

Thank you for your comment. I tested it a bit (even if the feature has been merged since a long time now…), I didn't notice any issues with the selectors I'm using !

The "HTML+XPath (Web scraping)" feature seems really cool ! I tested it a bit, it's very nice 👏 (and well, it may also deserve some way to validate / preview what was entered, in the future - it took me some time to have what I wanted 😅)

@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented Jun 1, 2022

Thanks for the feedback @javerous :-)

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.

5 participants