Bring the Drush integration up-to-date

Thanks for the patch!
However, I'm really not sure how this is at all different from search-api-reindex – except that it doesn't invoke the hook, but that can hardly be the whole point?
I think we should close this as "won't fix" – or does this really have a purpose?

In apachesolr and in native drupal search afaik it was introduced to support huge indexes on weak servers, to split big reindex for separate iterations.

Yes, I would like to get this in, it is a very useful command to reindex everything without deleting everything. Reindex is reindexing everything on the spot, and that might not be intended if the indexing is running during cron or other ways to index your data.

Yes, I would like to get this in, it is a very useful command to reindex everything without deleting everything. Reindex is reindexing everything on the spot, and that might not be intended if the indexing is running during cron or other ways to index your data.

Please look at the drush.inc file and the patch: what you describe is exactly what search-api-reindex does. It doesn't index any items (not even when "Index items immediately" is enabled), just marks all of them for indexing again. And this is what search-api-mark-all does, too, just without calling the proper hook.
So, while it might have made sense in D7, it doesn't seem to have any use anymore in D8.

Hm, then may be problem is with 'search-api-reindex' method, because semantically it means immediate clear the index and reindex from scratch. So then may be just rename it to mark-all and that's it? :)

Clearing the index is something different again, when you reindex the old data still stays in the index (until it is gradually overwritten by the newly indexed data).
But you're right, this was always a bit confusing, probably. However, "mark all" is also not really descriptive by itself – mark in what way? So search-api-mark-all-items-for-reindexing would be the really clear name here, but that's of course too verbose again. But maybe we can do a quick poll somewhere, and if most users agree that the current name is too confusing go with something like search-api-queue-reindexing. If the alias stays the same, hopefully it won't break that much (or cause that much confusion). Or, we could even set the old name as a second alias.

I'm also more in favor of search-api-mark-all. Having anything related to reindex or index suggest an action that would index things. Which is not the case here. In Search API we have trackers that mark these things so we could also do:

search-api-retrack-all
search-api-reset-tracker
search-api-mark-all
search-api-mark-all-for-indexing

In pure technical correctness, I like the search-api-reset-tracker, as that effectively resets the tracker information and it would reindex everything. It's not something you should do often and should come with some warnings. I also believe it should come with certain options so that it could reset an individual item for the tracker but also complete entity types etc if you want to reindex all those.

OK, then let's go with search-api-reset-tracker, add the old name as an alias, and maybe add options for marking only a single item or datasource, too (if easily possible).
And yes, this should also have a confirmation step, like we have in the admin UI.

But since I don't think that any of the other commands have a confirmation step either, maybe that should be a separate issue, "Add confirmation steps to Drush commands".

+++ b/search_api.drush.inc
@@ -88,38 +84,33 @@ function search_api_drush_command() {
+      'search-api-mark-all',

mark-all aliases should probably not be there anymore

Thanks for the new patch!
That has a bit of a larger scope, but I guess that's OK.
A few problems, though:

1. +++ b/search_api.drush.inc
   @@ -88,25 +84,33 @@ function search_api_drush_command() {
   +    'aliases' => array(
   +      'search-api-mark-all',
   +      'search-api-mark-all-for-indexing',
   +      'sapi-r',
   

   As Nick says, this should include the old search-api-reindex, mark-all-all-for-indexing is definitely unnecessary here.
   Including search-api-mark-all probably makes sense, though. And maybe the alias of that in D7, too? That way, all old scripts will still work, even though the two commands now do the same.

2. +++ b/search_api.drush.inc
   @@ -238,6 +239,7 @@ function drush_search_api_disable() {
   +    /* @var Index[] $indexes */
   

   Those should be removed.

3. +++ b/search_api.drush.inc
   @@ -303,16 +306,14 @@ function drush_search_api_status($index_id = NULL) {
   +  $indexes = search_api_drush_get_indexes(drush_get_option_list('index-ids'));
   

   Is that correct? It seems the index is still passed as an argument, not an option?

4. +++ b/search_api.drush.inc
   @@ -379,15 +380,30 @@ function drush_search_api_index($index_id = NULL, $limit = NULL, $batch_size = N
   +        $index->getTracker()->trackAllItemsUpdated();
   

   No. This should still use $index->reindex().

Thanks, looks better already. However, still two problems (one smaller one larger):

+++ b/search_api.drush.inc
@@ -396,12 +392,13 @@ function drush_search_api_reset_tracker($index_id = NULL) {
+        \Drupal::moduleHandler()->invokeAll('search_api_index_reindex', array($index, FALSE));

I wondered about that myself, but I don't think we should invoke the hook when we're not doing a complete re-indexing.
If we really want to, we should have an extra argument for the hook to pass the affected datasources. But at the moment, I'd just skip this hook.

+++ b/search_api.drush.inc
@@ -180,17 +181,17 @@ function drush_search_api_list() {
-  $index_ids = func_get_args();
+  $index_ids = search_api_drush_get_indexes($index_id);

That won't work, will it? $index_ids will contain already loaded entities, but later in the function you pass this again to search_api_drush_get_indexes() – and it will break.
Same for the other places you changed that. Please make sure to properly handle both cases (index ID passed or not) with your code.

Better, thanks!
However, this still had some problems. How about this patch? Could someone please try out whether all commands now work as expected? (A review of the patch would also be appreciated, of course.)

I'm committing this now, we can just see whether someone complains about things being broken.