Add wp-cli integration - Githubissues

matiasbenedetto commented 1 month ago

What?

Add wp-cli integration for theme-check plugin

Why?

To use the output of the theme-check plugin in automated environments like CI workflows.

How to use and how to test

In your WordPress install theme-check plugin using this branch.
Install wp-cli in your system.
Run in your console wp theme-check run.
Observe the output in your console

It should look like this:

Running wp theme-check run :

Running wp theme-check run --format=json:

Screencast

Screencast from 06-09-24 11:39:55.webm

jffng commented 1 month ago

Nice change! I can't push to your branch, but here is some suggestion changes to the readme:

Usage with wp-cli

To use with wp-cli, ensure the theme check plugin is active and wp-cli is installed. The theme-check subcommand is added to wp-cli and can be used as follows:

wp theme-check run [<theme>] [--format=<format>]

Options

Option	Accepts	Required	Default
`theme`	The slug of the theme to check	No	Current theme slug
`format`	`cli` or `json`	No	`cli`

Examples

wp theme-check run wp theme-check run twentytwentyfour wp theme-check run --format=json wp theme-check run twentytwentyfour --format=json

matiasbenedetto commented 1 month ago

@jffng cool, it looks good. I added you as a collaborator in the fork so you can push :)

codestylist commented 1 month ago

What a cool feature @matiasbenedetto . Thank you for this! Hope it will be in the master soon!

ernilambar commented 1 month ago

When I run check on a theme with errors. wp theme-check run mazebox. Output is:

Error: Theme check failed for mazebox.

But when there is no errors, output includes recommendation and info items.

Success: Theme check completed for mazebox.

RECOMMENDED: No reference to register_block_pattern was found in the theme. Theme authors are encouraged to implement custom block patterns as a transition to block themes.

RECOMMENDED: No reference to register_block_style was found in the theme. Theme authors are encouraged to implement new block styles as a transition to block themes.
...

matiasbenedetto commented 1 month ago

When I run check on a theme with errors. wp theme-check run mazebox. Output is: Error: Theme check failed for mazebox.

I don't know what kind of errors you are checking, but I think this is expected if the checking couldn't run. Theme-check will return an error message if the check can't be run for some reason (errors parsing theme code, for example). The success and error messages are only used to signal whether the plugin was able to run the checks or not, not to output the results of those checks.

But when there is no errors, output includes recommendation and info items.

It's a little bit counterintuitive, but the theme-check plugin doesn't work with the objective of outputting a boolean ("theme OK/ theme with errors"). The plugin is not designed like that. It just outputs a list of messages regarding the theme, but it never says whether this theme is OK or not. Maybe in the future, we could implement that, but that's not the case yet.

ernilambar commented 1 month ago

Example:

$ wp theme install twentysixteen --activate
Installing Twenty Sixteen (3.3)
Downloading installation package from https://downloads.wordpress.org/theme/twentysixteen.3.3.zip...
Using cached file '/Users/nilambarsharma/.wp-cli/cache/theme/twentysixteen-3.3.zip'...
Unpacking the package...
Installing the theme...
Theme installed successfully.
Activating 'twentysixteen'...
Success: Switched to 'Twenty Sixteen' theme.
Success: Installed 1 of 1 themes.

$ wp theme install twentyten --activate --quiet
$ echo $?
0

Another case:

$ wp theme list --format=json --quiet
[{"name":"twentytwelve","status":"inactive","update":"none","version":"4.3","update_version":"","auto_update":"off"}]

$ wp theme list --format=json
[{"name":"twentytwelve","status":"inactive","update":"none","version":"4.3","update_version":"","auto_update":"off"}]

There is no right or wrong approach. We can go for any approach we agree upon. But from my experience, general expectation in CLI is that if I pass --format=json then I would expect output in valid JSON format, whether quiet flag is passed or not.

vcanales commented 1 month ago

There is no right or wrong approach. We can go for any approach we agree upon. But from my experience, general expectation in CLI is that if I pass --format=json then I would expect output in valid JSON format, whether quiet flag is passed or not.

I see now. Yes, I agree that if we're explicitly asking for a format, it makes sense to bypass --quiet. On the other hand, these seem like incompatible flags to me; why would someone use both? This is an unrelated discussion, though.

ernilambar commented 1 month ago

Just an idea: What if we simply list check results like CLI displays. Below is one example. Each message is splitted into type and message. This way we could support different format using WP_CLI\Formatter(), like table, json, csv, yaml.

When I need to check in my CI runner where I just need to check required items or not, I could use:

wp theme-check run theme-slug --type=error --format=json

matiasbenedetto commented 1 month ago

I went through the theme-check plugin code again, and I found that we can get a boolean signaling if the theme has required changes or not. Knowing that I was able to I implemented all the reamaining feedback (thanks you all!) on the latest commits: