More and more fixes here and there...

[GwynethLlewelyn]

All right, bear with me, I'm slightly more confused than usual :)

I think these are the latest changes, at least so far. I understand that some of the "older" things I did two years ago got merged two days ago — thus the main source of my confusion! I most definitely should clean up the mess that I've made with Git by cleanly separating the branches. Alas! Living and learning! (I'm really, really just an amateur Gitter — I just know the basics of the basics, and also know how to search for help when things go really wrong).

Anyway, there are really, really, really lots of changes. Most, however, are just pretty-printing (yay for that...) and lots of DocBlocks. I've noticed that recently the WordPress team has been harsher on non-WP-compliant coding styles, mostly also because, these days, all you need is a good PHP editor, and integrate it with Intelephense, PHP Code Sniffer — with Automattic's own WordPress Coding Standards — and PHP Mess Detector, as well as Automattic's Coding Style Handbook for WordPress). For good measure — you can add some CI on GitHub using its Actions.

To make things nicer, the coding and documentation style of WordPress predates all those rules, norms and guidelines, and while there is quite a lot that is similar to some popular coding standards, others are the exact opposite: for instance, WordPress prefers snake_case instead of the far more popular camelCase... except for classes, where PascalCase is the established rule. Duh!

There are a lot of those annoying little things in their 'rules' and getting everything to validate is a real pain — even with automated tools (Code Sniffer can automate the whole procedure... in theory... if you trust it enough, of course! I don't).

So, no, I'm nowhere close to having done that. Like other contributors, the best I can do is tweak things here and there to be more conforming with the WP coding standards, especially when refactoring code, or fixing bugs. And, when I'm losing focus, I just do a few more methods here and there. But especially on the larger classes, I've barely scratched the top of the iceberg. Still, I guess that the WP tech team that is responsible for approving plugins will realise that FeedWordPress is really an old plugin, tracing its early beginnings to well before the WP coding standards were defined/invented, and it's a massive plugin, doing "everything but the kitchen sink" without relying upon any of the popular "frameworks" (most of which I hate with a vengeance) — which makes the plugin amazingly fast, of course, at the cost of making some things quite hard to maintain.

A typical example is... translations. As mentioned on my previous PR, there is a lot of text which needs to be translation-ready. And I really mean a lot, because FeedWordPress is exceptionally verbose on its dashboard (thus auto-documenting itself, in a sense). Because of the way the WP wp-admin was developed over two decades ago, there is no real separation between code and HTML, but everything is jumbled together — making things hard to maintain. Then there is plurals, and numbers, and so forth, all of which are supposed to be translation-ready as well. Again, I started doing some of those things as well, but there is really a lot left to be done. As said, now more than ever, there is no real alternative to FeedWordPress, so non-English speakers have no other choice but use it as it is. Patience...! Over the years, it can only get better and better.

Then... dealing with obsolescence. And we have two kinds to deal with: PHP is dropping the 7.X series, so, wherever I could, I bumped what I could find to make it compatible with PHP 8.2. There are excellent reasons for being full PHP 8-compliant — the JIT compiler is so much better, because PHP 8.X is even stricter than PHP 7 — but it requires thinking of PHP as less of a messy, type-less, interpreted language, and more as a quasi-compiled, almost-but-not-quite typed language. The latest round of changes at the PHP level now includes type declarations (an abomination under the Lord!... or maybe not), even if they're not strict but just guidelines; in fact, they convey approximately the same level of type hints that DocBlockr does — or PHPDoc, or whatever you call these structured blocks of comments before classes/methods/functions/variables.

Speaking strictly of myself, these are really useful — not so much for automatic generation of documentation (which it's what it was designed for!), but rather as hints to the Language Server Protocol — which, under PHP, is almost always Intelephense (because, well, it's the most complete, so it's the gold standard in terms of PHP LSP). This does make finding obscure things so much easier! For example, I found some 'broken' variables that I had no idea what they were for (mostly because they were of the a->-b->c kind, which, under PHP, is highly ambiguous, at least for a human looking at code they're unfamiliar with). But by searching through the whole code, I managed to track it down to the correct method & class, and, once that variable was correctly commented inside its DocBlockr, the LSP automagically inferred a lot of things, such as the many types that could or could not be used inside certain functions and structures — and started giving errors on a few methods which had been applied to the wrong object! (while running that code, it would silently fail with a few nulls here and there, which would be 'mostly harmless') Those automatic mechanisms are really useful and well worth the extra time in documenting the code properly, or at least as properly as time permits.

Anyway, this is just to explain why there are several spots where the actual change seems to be very minor and hardly worth the time in 'fixing' it. The truth is that I was paying close attention to what the LSP and Code Sniffer were finding out. A variable that is declared, but never used? (under some languages, such as Go, this also gives a compiler error!) Well, perhaps it's a typo, right? (In some cases, it was.) Or perhaps the code was refactored and that variable became irrelevant? (The most likely case.) Or — yuck! — the variable actually needs to be there, even if it's never used, because it's assigned to a method that requires something to be returned (once it finishes to handle a complex sequence of code), and which is supposed to be checked for potential errors — but these might be irrelevant and thrown inside the method anyway, so the variable is just there for the purpose of... calling a method and taking advantage of its side-effects!

This is a terrible way of programming, but, alas, when dealing with legacy code, it might have been the only way of doing things, a decade or so or more, and there is nothing we can do about it — except, possibly, to document that this variable will not be really used for anything whatsoever (that's why I like the programming languages that allow anonymous variables, for quickly disposing of those return values that we're not interested in; PHP has anonymous classes and methods/functions [e.g. closures], but no anonymous variables, as far as I know). Or, possibly, you can always print the result out as part of the diagnostics (which I have occasionally done, but not consistently so). There is a lot to be done in this regard; almost every PHP file in the plugin will have such "no-op" variables here and there, and it's just thanks to the automated tools flagging them that I decided to ake a closer look — and, very rarely, as said, I've figured out that these were tied to typo-induced bugs (which, however, did not stop execution with an error dump!). There is a lot to be said about those constructs that rely upon 'side-effects' which are not well-documented, but thst's neither here nor now.

I've also sprinkled a few more Dashicons here and there, and enhanced a bit the SVG sent before; now it should look a bit better — and it's also more compact in size. Good-bye, sprite-based GIFs from the late 1990s! (There might be many more still around.)

Known bugs

1. The most 'visible' one (pun intended): when clicking on the tab link that is supposed to only show Inactive links, the page refreshes but still shows the active links page. The variable 'visibility' is being correctly sent (and it's worth mentioning that it's fully conforming with the WP coding standards, where it properly uses the human-readabmanage to le 'Y' or 'N' as opposed to a true/false that might always be ambiguous...), but it seems to be ignored. This requires a bit more research, and it might be related to some code refactoring that I had expected to have correctly changed (but apparently not). Or perhaps this is an old, outstanding bug, which I just happen to notice now — I cannot say.
2. In the debugging dashboard page, there is a input box (for tweaking some timings) which is originally placed inside an argument that, in turn, gets sent to the WP text rendering processor — which encodes all HTML into harmless >s and their ilk. That means that the HTML code for that box is being spewed out as opposed to being rendered within the rest of the page — as it happens elsewhere on the dashboard, without such issues. I didn't manage to fix that; it's something TBD.

Anyway, last but not least, and again for the sake of formality, I fully release all the code, images, text and further resources that are included in this PR under a GPLv2 (or later) license, so you can do with these assets whatever you wish.

Happy code hunting!

Cheers,

• Gwyn

[GwynethLlewelyn]

Thanks for the PR approval, but... are you really sure that you want to approve them all without at least a cursory review? :-)

On the other hand, let me add those two "known bugs" I mentioned as GitHub issues, so that they don't get forgotten!