Add `tested_up_to` field

[tfirdaus]

This Pull Request is a PoC for addressing the issue reported in #241. While the changes made in this PR seems to be working, there are a couple of things I'd like to confirm:

1. Adding the "Tested up to" information requires requires parsing the plugin readme.txt. In this Pull Request, I've added a new trait largely taken from the Parser class from WordPress.org since I believe the WordPress.org team has likely dealt with various cases we might not be aware. But I've also made some adjustments to tailor it to this issue. I want to make sure if this approach is acceptable?
2. Parsing the readme.txt file may introduce some overhead and could potentially impact performance, especially when dealing with a large number of plugins. Should we consider making the field tested_up_to optional, similar to how we handle wporg_? This would mean that the file would only be parsed when the field is explicitly specified in the --fields parameter.

[swissspidy]

• Adding the "Tested up to" information requires requires parsing the plugin readme.txt. In this Pull Request, I've added a new trait largely taken from the Parser class from WordPress.org since I believe the WordPress.org team has likely dealt with various cases we might not be aware. But I've also made some adjustments to tailor it to this issue. I want to make sure if this approach is acceptable?

Can you elaborate on what adjustments you made? Does the class not support getting this information out of the box?

I don't like maintaining our own fork of this class. I'd much rather pull in https://github.com/afragen/wordpress-plugin-readme-parser via Composer for example.

• Parsing the readme.txt file may introduce some overhead and could potentially impact performance, especially when dealing with a large number of plugins. Should we consider making the field tested_up_to optional, similar to how we handle wporg_? This would mean that the file would only be parsed when the field is explicitly specified in the --fields parameter.

That makes sense to me.

[tfirdaus]

Can you elaborate on what adjustments you made? Does the class not support getting this information out of the box?

@swissspidy The class works fine but it is huge and parse part of the content that is not required in this case. So I took only the parts from the class that we need to get the headers, and adjust a few lines to adhere with the PHPCS in WP-CLI.

Thanks for bringing up that library; I wasn't aware of its existence. I believe we can rely on it if it's the preferable approach. I'll test it out and make the necessary updates.

[tfirdaus]

I noticed that the CI tests failed.

PHP Parse error: syntax error, unexpected '=' in vendor/afragen/wordpress-plugin-readme-parser/class-parser.php on line 701

The issue comes from the ??= operator in the afragen/wordpress-plugin-readme-parser dependency, which was only introduced in PHP 7.4. It seems it only copy from WordPress.org as is and does not maintain compatibility with older PHP versions.

🤔 There are a couple of things I could think of we could consider:

1. Install afragen/wordpress-plugin-readme-parser at a specific reference point. However, this might mean missing out on new updates or patches that are only available in the latest commit.
2. Create a fork and ensure compatibility with PHP versions supported by WP-CLI. If we are leaning to fork the package, should it be maintained under @wp-cli organization?

@swissspidy What's your thought?

[swissspidy]

Well then I guess we have to fork it after all 🤷 Can be a file in this repo again. Does not need to be a separate package.

[danielbachhuber]

I'd prefer not to fork something like this. Could we:

1. Ask Andy if he'd be willing to support older PHP versions for compat?
2. Conditionally make this feature only available for PHP 7.4+?

[swissspidy]

@danielbachhuber Andy is not maintaining it. The file is a 1:1 copy of what's on Meta with a regular sync.

[swissspidy]

Honestly personally I don't see much value in showing the "Tested up to" information anyway

[tfirdaus]

@swissspidy as I dig in this issue further, I've just realized it would require extra maintenance work, especially since the parser isn't compatible with older PHP versions and doesn't adhere to the PHPCS rules in WP-CLI. It seems like a significant effort for a feature that may only provide little value. And as mentioned ☝️ doing this will adds some overhead, which could slow down performance in some scenarios. @danielbachhuber, at this point, I'm not sure if we'd still want to pursue completing this issue.

[tfirdaus]

@swissspidy 👋 I'd like to revisit this Pull Request since and push this forward if possible.

One thing that makes this update a bit more complicated was due to the external dependency to parse the readme files. It is a direct copy of the one from WP.org, is limited to specific PHP versions, and there are a few files and dependencies there that simply copying would be inconvenient.

So I decided to create a new package syntatis/wp-plugin-readme-parser. It is similar to afragen/wordpress-plugin-readme-parser, but I've included the original Markdown class and made adjustments to ensure compatibility with PHP 7.0.

Since it's Hack Day, I wanted to share it here to get some feedback and to see if using this package aligns with our approach for this issue. Thanks!

[danielbachhuber]

@tfirdaus Do we need an entire dependency to pull the "Tested up to" value? I think I'd prefer some simple regex to avoid all of the complexities of a dependency.

[tfirdaus]

@danielbachhuber initial plan was actually quite simple. But thought on the discussions above ☝️ we prefer not maintaining this on our own and use an external library. Initial updates can be found 👉 https://github.com/wp-cli/extension-command/pull/413/commits/12c5b34fe16f1e9590cce754c8b8fd47a8861292#diff-3df1a510df0bb92bdb36e3816eab6cb8d8a761db550713f56f19560762bf9a76

[danielbachhuber]

@tfirdaus Is there anything preventing the regex approach?

[tfirdaus]

@danielbachhuber nothing preventing using a simple regex. I only thought that if we could have close implementation as how WP.org parse the readme file. I could try it with the regex and see how it goes :)

[tfirdaus]

@danielbachhuber At the moment, this update only reads the readme.txt file to keep things straightforward. I'm a bit hesitant about adding support for readme.md. Unix file systems are case-sensitive, and there isn't a clear convention for file naming in the WP.org repository for the readme.md file that I could find. Should we expect it to be in lowercase like (readme.txt), or in uppercase (README.md) like it often is in GitHub repos? Should we support both conventions?

[danielbachhuber]

@tfirdaus Let's just support readme.txt for now.

[tfirdaus]

@danielbachhuber sounds good 👍