Closed rookiejet closed 4 months ago
Ooof I did not catch that. Hope I'll be able to sit down this weekend to look at it fairly sure it's a matter for removing https://github.com/feediron/ttrss_plugin-feediron/blob/a586a2a96ceb4760d3c2e74d6968129f1b6caca8/init.php#L471 and iterating the maxpages counter.
@monofox any thoughts?
I can reproduce it. But looking deeper into that part of links array, is fairly interesting. I'll try to understand the algorithm tomorrow evening. Because in this case, the link is not reformatted, as the option should be disabled per configuration.
Just for you reference. Replacing line https://github.com/feediron/ttrss_plugin-feediron/blob/a586a2a96ceb4760d3c2e74d6968129f1b6caca8/init.php#L457
and reverting with non-referencing $lnk
, will solve. But then the $links array have different value in case of executing reformat. Thats what i mean with understanding the correct algorithm.
@dugite-code : i've made a proposal in #202
Please let me know your opinion on this.
What i've faced during testing the different scenarios, that the counter and the links will not proper contain the right values while iterating in recursive mode. The counter is only increased in a next call. So if a page contains 4 links, all subsequent 4 links are considered as page 2. This might be an issue in proper taking care of $maxpages setting. Due to replacing, than merging and extending the $links-Array, the seenlinks is never containing all subsequent previous links, as the its replaced with its own. But this is not relevant here for solving this.
After #199 recursive multipage option fails for the following recipe
Fetches for multipage articles using that recipe now error out with a memory exhaustion error.
I reverted the change to this line in commit https://github.com/feediron/ttrss_plugin-feediron/pull/199/commits/d0cd6a052cb78a96f40a366273ce5a3a765a6e95 and the above recipe is working again.
Article tested: https://arstechnica.com/gadgets/2024/05/all-the-ways-streaming-services-are-aggravating-their-subscribers-this-week/