Move drush integration from views into drush

Should this be against Drush?

Sure, I'll take this into Drush core. I looked at the current Views Drush code and it needs

1. Support --simulate by running config->set and other "destructive" code through drush_op().
2. Add completion support for arguments
3. return your data in addition to printing it for the benefit of --backend calls
4. When declaring options, you can state if a value is optional and give an example value. For example, see options at config-set command at http://api.drush.org/api/drush/commands!core!config.drush.inc/function/c...

@moshe weitzman Thanks! I will check out your pointers later and try to get a patch together soon. api.drush.org has been updated too :)

I think this covers the points in #3.

Thanks. Some more tweaks ...

You can remove the hook_drush_help() entries except the meta: ones. When missing, Drush uses the description in the command definition which is sufficient here.

When you use those options that are declared with arrays, a value is required so change value' => 'optional', to 'required'.

'example-value' => 'enable',. typo: enabled. or at least thats what the other help lines use.

drush_views_analyze should return its data in addition to printing.

drush_views_disable and drush_views_enable have empty argument handling but thats now done by the command definition. Please remove.

The list and analyze commands should support the --format option. See drush_config_list() for an example. Remember to add this option to the command definitions as well.

Thanks for another review. I think I've addressed all those.

It's funny, because I actually wrote the drush_config_list command! so I should have seen that coming.

Looks good to me. WIll commit this in a day or two. More feedback is welcome.

Great, thanks.

Committed to master. Will be in 6.x release of Drush. Thanks.

Is Drush 6.x for Drupal-8 only? The commit in #11 breaks Drush for Drupal 7.x with Views enabled. If we want to support Drupal 7 &/or Drupal 6 in Drush 6, then we'd need a mechanism to avoid including commands/core/views.drush.inc for Drupal < 8. In the short term, I think that D6 & D7 support in master is desirable, but long-term that might not be practical. If D6 & D7 are unsupported, then we should fail-fast in the bootstrap.

I guess the other question I have is, would it hurt to put views.drush.inc in Drupal 8 core? Then the right version would be selected automatically.

It wouldn't hurt, but core has always had a habit of pretending like Contrib doesn't exist in order not to play favorites (or something). It would be fantastic if we could move lots of drush commands to core Drupal but I think thats unlikely.

Drush 6 still want to be compatible with D6 and D7 so this is a bug.

I agree with moshe, drush command files moving to core is very unlikely. Could we have some logic where the list of command files are fetched that are included; So if there is a command file of the same name in contrib it can overrride the core file before it is included? Maybe it would be in command.inc, I'm not sure without having a proper look.

EDIT: or can we do something with version numbers to include the correct files? Like 'views_8.drush.inc'.

Wrote my patch before I read #15; made the pattern "views.d8.drush.inc". (The suggestion for contrib overriding Drush core won't work without a large change to the Drush bootstrap, as the Drush core commandfiles are included before Drupal is even bootstrapped.)

Note that the requisite rename of "views.drush.inc" to "views.d8.drush.inc" is not included in this patch. Only lightly tested -- works with Drupal 7. :)

So using drush6 and drupal7 will cause the version specific commandfile to load and the non-version specific will not load? I guess thats a better solution than requiring drush commandfiles to work for all supported drupal versions (i.e. edit views.drush.inc so it works for d6 and d7). is it worth coming up with a syntax like views.d8+.drush.inc in order to signify that the commanddfile should work for drupal9 as well? we'd want to same for drush major version-specific config/alias/commandfiles.

In d6 and d7, views.d8.drush.inc in Drush core will not load, and whatever Drush include file is in the View module will load. In d8, the views.d8.drush.inc will load, and there will be no non-version specific file in the Views module. I have not tested what will happen if there are two version-specific files under consideration, but I expect it will work.

I started to support views.d8+.drush.inc (in regex only), but I think it's perhaps not worth it. The easy workaround would be to make views.d9.drush.inc include views.d8.drush.inc if they were compatible.

Be sure to run drush cc drush before applying the patch above, since I changed the format of the commandfile cache. I did not test what happens if you don't do this; probably all commands fail with php errors. Might be best to put the cache format back the way it was, or change the cache filename pattern so the two can never conflict.

So, I think if we do not support views.d8+.drush.inc, then we can reduce our considerations to just X.drush.inc and X.dMAJOR.drush.inc, and simplify #16 quite a bit.

Here, this is simpler. Again, remember to git mv commands/core/views.drush.inc commands/core/views.d8.drush.inc and drush cc drush before testing.

For me, this patch seems to be working for d7 but not for d8 sites. Does D8 work for you? Otherwise, it looks good. If/when everyone agrees I guess the file views command file can be renamed in this patch too?

Haven't gotten to d8 testing yet. In theory, if the Drupal version does not match, the commandfile should go in the 'deferred' state, and be tried again at a later phase. I think this worked in #16 (but again, didn't really test it thoroughly), but it looks like I probably broke that in #20. Will test more as I have time.

Still haven't tried on d8, but I made myself a "foo.d7.drush.inc" file and tested successfully with d7, fixing a problem that existed in #20 and #16. Should work on d8 too.

I haven't tested in detail but the patch in #23still results in a fatal error on both 6 and 7 sites that have views installed when running drush commands.

PHP Fatal error: Cannot redeclare views_drush_help() (previously declared in /.../drush/commands/core/views.drush.inc:14) in /.../sites/all/modules/views/drush/views.drush.inc on line 28

To save space, I omitted the rename of views.drush.inc in #23. To test it, run drush cc drush, apply the patch, then git mv commands/core/views.drush.inc commands/core/views.d8.drush.inc. I could provide a formatted patch with the rename if desired.

Here it is with the moved file, just to make it easier for testers.

This can go in whenever Greg is satisfied with it. Also, needs docs wherever we talk about commandfile naming.

Committed #23 + some docs. d8 install failed for me, so did not test there; will try again later. Re-open if any problems are found.

Oh, committed to 6 only; presume we don't want this in 5. Should be backwards-compatible enough to do so if desired, though.

Now I am having trouble with 'dl', and git bisect says it was my commit. :(

Fixed with commit 06b1302.

There is a flaw in drush_get_context; I will discuss this in a separate issue later.

c.f. #1816192: Beware the poison pill: flaw in drush_get_context can expose Drush to mysterious failures