ideas icon indicating copy to clipboard operation
ideas copied to clipboard

Add ranges to commands that accept one or more IDs

Open schlessera opened this issue 7 years ago • 7 comments

For commands that accept one or more IDs as positional parameters, I would like to investigate adding ranges and partial ranges as valid IDs.

A range would take the following form: <range start>-<range end>. This would mean that all existing IDs that fall into the range that starts at <range start> and ends at <range end> would be included.

Example: 15-35 would mean all IDs starting from 15 and going up to and including 35 would be processed. Only valid existing IDs within that range would be processed, so that no warnings are being thrown for missing IDs in-between.

A partial range would take one of these forms: <range start>- or -<range-end>. This would mean that either all existing IDs starting from <range start> without any upper bound or all existing IDs from 1 onwards up to and including <range end> would be included.

Ranges and individual IDs can be combined in any way, so you can use something like the following: 3 7 12-24 19 34-.

Commands that could profit from this range notation:

  • comment approve
  • comment delete
  • comment recount
  • comment spam
  • comment trash
  • comment unapprove
  • comment unspam
  • comment untrash
  • comment update
  • embed cache clear (*)
  • embed cache trigger (*)
  • media regenerate
  • post delete
  • post meta delete (*)
  • post meta update (*)
  • post update
  • site activate
  • site archive
  • site deactivate
  • site delete (*)
  • site mature
  • site private
  • site public
  • site spam
  • site unarchive
  • site unmature
  • site unspam
  • term delete
  • term meta delete (*)
  • term meta update (*)
  • term update (*)
  • user add-cap (*)
  • user add-role (*)
  • user delete
  • user meta delete (*)
  • user meta update (*)
  • user remove-cap (*)
  • user remove-role (*)
  • user reset-password
  • user set-role (*)
  • user spam
  • user unspam
  • user update
  • widget delete
  • widget move (*)
  • widget update (*)

(*) => not variadic yet

schlessera avatar Jul 02 '18 08:07 schlessera

@danielbachhuber, @johnbillion Would love your thoughts on the above.

I know that we can already use xargs for doing similar things, but I do think this makes it much easier for people to execute for complex operations, and lets you more easily fine-tune the IDs you're targeting.

schlessera avatar Jul 02 '18 08:07 schlessera

I'm 👎 on this for a few reasons:

  1. It violates our philosophy of composability.
  2. It would be new syntax for the user to learn.
  3. It's valuable for a WP-CLI user to learn to use xargs and other shell tools in conjunction with WP-CLI.

danielbachhuber avatar Jul 02 '18 11:07 danielbachhuber

I'm on the fence.

  • Does this syntax provide real value? What use cases does this improve? Some example use cases would be good.
  • Can it be made performant internally? More so than piping in IDs with xargs?

I don't think this affects composability. The output presumably would not vary from what's already output when using a variadic list of IDs, and the input would still be composable from other commands.

johnbillion avatar Jul 02 '18 12:07 johnbillion

Responding to @danielbachhuber's cons:

  1. It violates our philosophy of composability.

Composability is still available, but extended so that you can also compose ranges. So, you can do something like this:

wp post delete $(command to figure out start)-$(command to figure out end)

I don't see this as breaking that principle, it is a mere extension to make the syntax more expressive.

  1. It would be new syntax for the user to learn.

Yes, I agree. I don't think this would be a big issue, because a) the syntax is actually far from complex and b) it is purely optional, you can still just use one regular ID without ever noticing something was changed.

  1. It's valuable for a WP-CLI user to learn to use xargs and other shell tools in conjunction with WP-CLI.

I agree with that, but I don't think skipping a valuable performance and expressiveness improvement for that sake is a good move or a service to users. For a lot of users, this syntax change might actually mean they will never even have a need for xargs and don't need to deal with its much more complex syntax.

Responding to @johnbillion's questions:

Does this syntax provide real value? What use cases does this improve? Some example use cases would be good.

The original use case that triggered this idea is https://github.com/wp-cli/media-command/issues/82

Can it be made performant internally? More so than piping in IDs with xargs?

It will actually be more performant than the xargs right out of the box, because the IDs can be retrieved with 1 query to add them to the loop. So, a range like 1-300 will entail 1 query and 1 loading of WordPress only.

schlessera avatar Jul 02 '18 15:07 schlessera

Composability is still available, but extended so that you can also compose ranges. So, you can do something like this:

I don't see this as breaking that principle, it is a mere extension to make the syntax more expressive.

Sorry, I meant the other part of that philosophy:

A corollary of this is that commands should be orthogonal, which means that there should be no overlapping functionality between commands.

This applies to WP-CLI <-> general CLI utilities too. No sense duplicating a feature that xargs already offers, etc.

Generally, this proposed enhancement seems like a "clever" solution in search of a problem. It's not clear to me what real-world user need it solves.

danielbachhuber avatar Jul 02 '18 15:07 danielbachhuber

Both xargs and composing into a strings of IDs have limitations.

With xargs you have the problem that you're hitting the server with hundreds or thousands of WordPress requests, as it spawns a new WP-CLI process, and thus loads a new instance of WordPress for each separate ID. With a range like 1-5000, you only have 1 DB query to find IDs, and 1 WordPress process that does the operations.

With normal composition (wp post delete $(some command to fetch ids)), you have the problem that you might run into the argument list too long error. Different shells have different limitations as to how many argument characters they support.

A corollary of this is that commands should be orthogonal, which means that there should be no overlapping functionality between commands.

I get what you mean by that. However, this principle is only enforceable within reason. For a simple example, wp post delete, wp query and mysql and php are all valid ways of deleting a post (with the proper arguments) and thus do already overlap.

For a command like wp post delete <IDs>..., xargs is actually not what we are overlapping with, but the composition of wp post delete $(wp this or that) to fetch the correct IDs. And still, a simple wp post delete 10 11 12 13 14 15 16 17 18 19 20 is perfectly acceptable, without using composition. So, why would wp post delete 10-20 not be acceptable?

We're not introducing a new command, we're only letting the concept of "lists of IDs" understands ranges in addition to individual entries.

schlessera avatar Jul 02 '18 16:07 schlessera

Thanks for the clarification, @schlessera. I appreciate the additional detail you've shared.

I'm still opposed to the idea, but not strongly so. If it's something you want to pursue, then I'm amenable to that.

danielbachhuber avatar Jul 02 '18 17:07 danielbachhuber