wp-cli / doctor-command

Diagnose problems within WordPress by running a series of checks for symptoms
MIT License
146 stars 22 forks source link

Is the count format working as expected? #187

Closed BorisEvenMedia closed 7 months ago

BorisEvenMedia commented 7 months ago

Hi,

I am currently testing doctor in order to use it in a bash script.

And I'm afraid either I don't understand how the --format=count works or it's not working as intended.

On my test site I have :

When I try the check commands without parameters, I get the following results which are correct:

wp doctor check core-update
+-------------+---------+-------------------------------------+
| name        | status  | message                             |
+-------------+---------+-------------------------------------+
| core-update | success | WordPress is at the latest version. |
+-------------+---------+-------------------------------------+
wp doctor check plugin-update
+---------------+---------+-----------------------------------+
| name          | status  | message                           |
+---------------+---------+-----------------------------------+
| plugin-update | warning | 5 plugins have updates available. |
+---------------+---------+-----------------------------------+
wp doctor check theme-update
+--------------+---------+------------------------+
| name         | status  | message                |
+--------------+---------+------------------------+
| theme-update | success | Themes are up to date. |
+--------------+---------+------------------------+

But If I execute the exact same commands with the parameter --format=count

I always get the same result : 1

It seems that the count fromat only return 1 or 0 on error (I guess?).

I was expecting more something like the following (to match the results listed previously):

wp doctor check core-update --format=count
O
wp doctor check plugin-update --format=count
5
wp doctor check theme-update --format=count
0

Did I missing understand the word "count"?

swissspidy commented 7 months ago

Hmm I see how that's unintuitive/misleading.

Usually in WP-CLI, --format is for formatting a list of items, like as CSV or as a table. count is for counting the number of items in the list.

The wp doctor check command is mis-using this formatting a little bit and always displays the result in a table with just one item.

Typically elsewhere in WP-CLI we would do this instead:

$ wp doctor check plugin-update
Warning: 5 plugins have updates available.

$ wp doctor check theme-update
Success: Themes are up to date.

Then, a count option would be more useful (though right now not every check does a count).

Such a change now would be quite a breaking change.

@danielbachhuber Do you recall why this unusual output format was chosen?

mrsdizzie commented 7 months ago

Maybe this was added specifically for the --all option which would have more than one result: https://github.com/wp-cli/doctor-command/issues/124

Though there isn't a specific use case in that issue other than "it doesn't accept --format=count"

$ wp doctor check --spotlight --all
Running checks  100% [========================================================================================================================================================================================================================================================] 0:02 / 0:03
+--------------------+---------+-----------------------------------------------------+
| name               | status  | message                                             |
+--------------------+---------+-----------------------------------------------------+
| cache-flush        | warning | Use of wp_cache_flush() detected.                   |
| plugin-deactivated | warning | Greater than 40 percent of plugins are deactivated. |
| plugin-update      | warning | 2 plugins have updates available.                   |
| theme-update       | warning | 3 themes have updates available.                    |
| php-in-upload      | warning | PHP files detected in the Uploads folder.           |
+--------------------+---------+-----------------------------------------------------+
$ wp doctor check --spotlight --all --format=count
5

I don't know that it makes sense to use --format=count for a single doctor check ever though. I think for the example use case you would normally use something like:

wp plugin list --update=available --format=count
5

And core / themes have similar commands.

Even if doctor didn't output a table, I don't think count would be able to return anything other than 1 for a single doctor check (assuming they all output a single line message)

danielbachhuber commented 7 months ago

Do you recall why this unusual output format was chosen?

I don't think there was a reason. It was probably just my preference at the time.

BorisEvenMedia commented 7 months ago

Sorry I didn't know the it was possible to simply use :

wp plugin list --update=available --format=count

As the --update option isn't listed in the documentation: https://developer.wordpress.org/cli/commands/plugin/list/

But it seems to do exactly what I need. Thanks!

swissspidy commented 7 months ago

As the --update option isn't listed in the documentation:

It's covered by the [--<field>=<value>] Filter results based on the value of a field. line.

That means you can filter by any of the available fields