[renatonascalves]

fixes #18

[dlh01]

I'm not sure I follow the motivation for this change as described in #18. First, I don't follow how verbose mode is incompatible with formatted data. Second, since this PR only changes the name of the flag, not any behavior, I'm not sure I understand how it fixes the issue.

More generally, a --verbose flag is a well-established convention, while a --progress-bar flag is not so common. Combined with the above points of confusion, I'm not sure what benefits are being gained for this tradeoff.

Having said all that, if you think this is a needed change, I trust your judgment. The only other thing I would point out would be that I think this warrants the next version of the package being a major version, since this PR changes a public API (while #17 is arguably a bug fix).

[renatonascalves]

So, let me elaborate a bit more then.

We use --verbose to show debugging information. See https://clig.dev/#arguments-and-flags
This is also how it is used in WordPress core CLI commands. See this and this for examples.

In this command, we don't show any debugging information, it is being used to set PHP_CLI_Progress_Bar and show the progress bar. The usefulness of this is debatable, but it might be useful for sites with large databases and it is available with its proper flag.

The progress bar "convention" is something we created as part of https://github.com/alleyinteractive/wp-bulk-task/wiki. Mainly because the feature is usually used with migrations and migration takes time. One might want to keep track of how a command is doing.

So, #16 is not a bug fix since the command runs correctly with or without it. The error is that we are using a long running command but we do not print any data as part of collecting the report, and then we list the output in the format we want.

The cursor feature would only be relevant if we were printing something as the command is being run, as the data is being fetched and created. Because if the command failed for any reason, we would not lose any already fetched data for the reports. But for listing data, even if the command fails with 50% of the report, we lose all data with or without this --verbose flag.

See my talk about running WP CLI Commands At Scale where I went deeper into this.

So, to recap:

• debug is not being used correctly here since we are not adding any debugging information;
• we are allowing the user to see the progress bar with the correct flag (our convention);
• both Do not save the cursor to the database #16 and this one will be deployed in a major version change since I have more prs coming.

:)

[dlh01]

Thank you for this thoughtful response!

We use --verbose to show debugging information. See https://clig.dev/#arguments-and-flags This is also how it is used in WordPress core CLI commands. See this and this for examples.

Speaking just for myself, this definition of --verbose is unfamiliar, but that's just me. I'm also not sure that the links you include demonstrate that --verbose is used to show debugging information.

The guide at https://clig.dev/#arguments-and-flags says:

If a piece of output serves only to help you (the developer) understand what your software is doing, it almost certainly shouldn’t be displayed to normal users by default—only in verbose mode

Which, as I read it, only suggests that debugging output should be shown only when verbose mode is enabled, not that verbose mode should include debugging output.

Additionally, I would observe the authors of that document list a dedicated --debug flag as a standard flag for showing debugging output.

The two core CLI commands document their --verbose flags as:

Log useful information to STDOUT.

Prints rows to the console as they’re updated.

I would argue that neither description indicates that debugging output will be shown. ("Log useful information" comes close to my usual definition of the --verbose flag, incidentally.)

Moreover, WP-CLI itself contains a global --debug flag. As a command author, that's where I would send my debugging information. As a user, that's what I would expect to use to show debugging output.

In this command, we don't show any debugging information, it is being used to set PHP_CLI_Progress_Bar and show the progress bar. The usefulness of this is debatable, but it might be useful for sites with large databases and it is available with its proper flag.

The progress bar "convention" is something we created as part of https://github.com/alleyinteractive/wp-bulk-task/wiki. Mainly because the feature is usually used with migrations and migration takes time. One might want to keep track of how a command is doing.

I think the fact that, in this command, --verbose happens to show the Bulk Task progress bar is an implementation detail. It's information we happen to have available via Bulk Task that we can use to fulfill the user's request to be verbose. But what information gets included in the verbose output remains in our control as the command developers.

I would not expect, as a developer, that if I decided to change the internals of the command to not use Bulk Task, that I would have to find a way to rewrite the progress bar just to maintain compatibility with the flag. Instead, I would expect to find some other relevant way to be verbose.

So, #16 is not a bug fix since the command runs correctly with or without it. The error is that we are using a long running command but we do not print any data as part of collecting the report, and then we list the output in the format we want.

The cursor feature would only be relevant if we were printing something as the command is being run, as the data is being fetched and created. Because if the command failed for any reason, we would not lose any already fetched data for the reports. But for listing data, even if the command fails with 50% of the report, we lose all data with or without this --verbose flag.

If the command failed at 50%, then the next time it ran, it would check only the remaining 50% of the data, right? So it would appear to "complete," but the user would get results that were actually incomplete (because, as you say, it doesn't print anything until the end)? If so, that's what I mean by bug.

[dlh01]

🌳 For the reasons I mentioned, I'm still not quite convinced by this change, but the code itself looks good, and there's a method behind it, and that's important!

[renatonascalves]

I see I mixed up debug with verbose in the link I shared. Sorry.

That's tricky, because --verbose in your scenario is the progress bar where this convention is not set anywhere. So I'd say either way is confused. =P The fact that you and I read it differently is proof of that.

I'd only argue that my suggestion better aligns to how Alley developers use it. A --progress-bar to show a progress bar is how I write long-running CLI commands. It's not a WP core convention since their commands are not built to be long-running.

I think the fact that, in this command, --verbose happens to show the Bulk Task progress bar is an implementation detail. It's information we happen to have available via Bulk Task that we can use to fulfill the user's request to be verbose. But what information gets included in the verbose output remains in our control as the command developers.

I agree with this comment. However, the --verbose mode, in IMO, is being used incorrectly here. For the same reason I changed the cursor to stop saving the field in the database.

Unless we decide to actually have useful information in --verbose mode to this command, we should remove it since the usefulness of having the progress bar as the verbose output is debatable. Also, IMO, confusing (a user might ask: why don't I get any information, I get the progress bar instead? Is it doing something? Is it broken?).