Add skip-tables, structure-tables and tables to @optionset_table_selection

[webflo]

The three settings are missing in optionset_table_selection. Drush does not load the options otherwise.

@greg-1-anderson Should i add the @option annotation for these options? I think they should not be visible in the help section of the command. Because its not clear how to define an array via CLI. I saw other commands have @hidden-options. What you i use?

It works fine without the annotation anyways.

I have checked the examples in example.drush.yml and it works as expected with this small fix. 👍

[greg-1-anderson]

I get this result from help:

$ ./drush sql-dump --help
 [warning] DOMDocument::createTextNode() expects parameter 1 to be string, array given XmlDescriptor.php:257

Not sure why it's doing that. Probably because the default value is being provided by my drush.yml, and the default value is an array, and the help code can't handle that.

Unfortunately, "hidden options" are aspirational; these are not supported by Symfony Console at the moment.

[greg-1-anderson]

I am going to suggest that we simply make these defaults be some config item that is not an option. In other words, instead of:

command:
  sql:
    options:
      tables:
        common:
          - user
          - permissions
           - role_permissions
           - role
      structure-tables:
        common:
          - cache
          - 'cache_*'
          - history
          - 'search_*'
          - 'sessions'
          - 'watchdog'
      skip-tables:
        common:
          - 'migration_*'

we could instead define:

sql:
  tables:
    common:
      tables:
        - user
        - permissions
         - role_permissions
         - role
      structure:
        - cache
        - 'cache_*'
        - history
        - 'search_*'
        - 'sessions'
        - 'watchdog'
      skip:
        common:
        - 'migration_*'

I'm not sure about sql.tables.common.tables vs something else (sql.tables.common.list?), but in general it's better to just keep these out of options entirely, which avoids the issue of hiding them, help defaults, etc.

Note that arbitrary config may be set via -D, e.g. -Dsql.tables.common.structure='cache_*'; currently this does not support arrays, but perhaps we could consider an enhancement to allow that.

[webflo]

Can you give me an example for a command with "some config item"?

[greg-1-anderson]

This is still pretty rare, but site:set uses config to access a runtime value:

https://github.com/drush-ops/drush/blob/master/src/Commands/core/SiteCommands.php#L47

[weitzman]

Are we sure that these 3 options shouldn't be shown in help? Wouldnt it be useful to supply a comma delimited list of tables via the CLI? I think thats what these options are for.

[webflo]

@weitzman. --skip-tables-list etc. is for this purpose

$ drush sql-dump --help

...
  --skip-tables-list=SKIP-TABLES-LIST           A comma-separated list of tables to exclude completely.
  --structure-tables-list=STRUCTURE-TABLES-LIST A comma-separated list of tables to include for structure, but not data.
  --tables-list=TABLES-LIST                     A comma-separated list of tables to transfer.

[greg-1-anderson]

+1 LGTM.

We could potentially rename --skip-tables-list to just --skip-tables now that the later is no longer an option; however, I'm not sure it's worthwhile, given that it would impact a bunch of scripts, and these options probably aren't typed often.

[webflo]

Awesome. Thanks!