New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leverage symfony/console package #88

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
9 participants
@patcon
Contributor

patcon commented Sep 11, 2013

Just messing around, but thought I'd share in case anyone else wants to jump in.

References:

@greg-1-anderson

This comment has been minimized.

Show comment
Hide comment
@greg-1-anderson

greg-1-anderson Sep 11, 2013

Member

I considered this a while ago, but I am not convinced that this is any better than what we have.

Still need to answer questions such as how does this interface with output formats (note that drush_core_status returns an array, not a block of text), how do commandfiles declare additional commands, how do engines work (updatexml for different versions of Drupal), etc.

Member

greg-1-anderson commented Sep 11, 2013

I considered this a while ago, but I am not convinced that this is any better than what we have.

Still need to answer questions such as how does this interface with output formats (note that drush_core_status returns an array, not a block of text), how do commandfiles declare additional commands, how do engines work (updatexml for different versions of Drupal), etc.

@patcon

This comment has been minimized.

Show comment
Hide comment
@patcon

patcon Sep 12, 2013

Contributor

Oh god, they greg, but lots wrong and broken with test code so far. Just wanted to open a pull request with my hack-code so other folks knew someone was tinkering with it and that there's some basic boilerplate code if they want to go all-out.

I'll probably be picking away at it. Might see if there are more flexible cli helper libs besides Symfony/Console...

cc: @fabpot @jfsimon @Seldaek

Contributor

patcon commented Sep 12, 2013

Oh god, they greg, but lots wrong and broken with test code so far. Just wanted to open a pull request with my hack-code so other folks knew someone was tinkering with it and that there's some basic boilerplate code if they want to go all-out.

I'll probably be picking away at it. Might see if there are more flexible cli helper libs besides Symfony/Console...

cc: @fabpot @jfsimon @Seldaek

@patcon

This comment has been minimized.

Show comment
Hide comment
@patcon

patcon Sep 12, 2013

Contributor

Seems that Helper\DescriptorHelper deals with output formats:
https://github.com/symfony/Console/blob/master/Helper/DescriptorHelper.php

➜  symfony-console-demo git:(master) ✗ git clone https://github.com/havvg/symfony-console-demo.git --init --recursive & cd symfony-console-demo
➜  symfony-console-demo git:(master) ✗ git submodule foreach git pull
➜  symfony-console-demo git:(master) ✗ php app/console --xml help
<?xml version="1.0" encoding="UTF-8"?>
<command id="help" name="help">
  <usage>help [--xml] [command_name]</usage>
  <description>Displays help for a command</description>
  <help>The &lt;info&gt;help&lt;/info&gt; command displays help for a given command:

   &lt;info&gt;php app/console help list&lt;/info&gt;

 You can also output the help as XML by using the &lt;comment&gt;--xml&lt;/comment&gt; option:

   &lt;info&gt;php app/console help --format=xml list&lt;/info&gt;</help>
  <aliases/>
  <arguments>
    <argument name="command" is_required="1" is_array="0">
      <description>The command to execute</description>
      <defaults/>
    </argument>
    <argument name="command_name" is_required="0" is_array="0">
      <description>The command name</description>
      <defaults>
        <default>help</default>
      </defaults>
    </argument>
  </arguments>
  <options>
    <option name="--xml" shortcut="" accept_value="0" is_value_required="0" is_multiple="0">
      <description>To output help as XML</description>
    </option>
    <option name="--help" shortcut="-h" accept_value="0" is_value_required="0" is_multiple="0">
      <description>Display this help message.</description>
    </option>
    <option name="--quiet" shortcut="-q" accept_value="0" is_value_required="0" is_multiple="0">
      <description>Do not output any message.</description>
    </option>
    <option name="--verbose" shortcut="-v" accept_value="0" is_value_required="0" is_multiple="0">
      <description>Increase verbosity of messages.</description>
    </option>
    <option name="--version" shortcut="-V" accept_value="0" is_value_required="0" is_multiple="0">
      <description>Display this application version.</description>
    </option>
    <option name="--ansi" shortcut="" accept_value="0" is_value_required="0" is_multiple="0">
      <description>Force ANSI output.</description>
    </option>
    <option name="--no-ansi" shortcut="" accept_value="0" is_value_required="0" is_multiple="0">
      <description>Disable ANSI output.</description>
    </option>
    <option name="--no-interaction" shortcut="-n" accept_value="0" is_value_required="0" is_multiple="0">
      <description>Do not ask any interactive question.</description>
    </option>
  </options>
</command>
Contributor

patcon commented Sep 12, 2013

Seems that Helper\DescriptorHelper deals with output formats:
https://github.com/symfony/Console/blob/master/Helper/DescriptorHelper.php

➜  symfony-console-demo git:(master) ✗ git clone https://github.com/havvg/symfony-console-demo.git --init --recursive & cd symfony-console-demo
➜  symfony-console-demo git:(master) ✗ git submodule foreach git pull
➜  symfony-console-demo git:(master) ✗ php app/console --xml help
<?xml version="1.0" encoding="UTF-8"?>
<command id="help" name="help">
  <usage>help [--xml] [command_name]</usage>
  <description>Displays help for a command</description>
  <help>The &lt;info&gt;help&lt;/info&gt; command displays help for a given command:

   &lt;info&gt;php app/console help list&lt;/info&gt;

 You can also output the help as XML by using the &lt;comment&gt;--xml&lt;/comment&gt; option:

   &lt;info&gt;php app/console help --format=xml list&lt;/info&gt;</help>
  <aliases/>
  <arguments>
    <argument name="command" is_required="1" is_array="0">
      <description>The command to execute</description>
      <defaults/>
    </argument>
    <argument name="command_name" is_required="0" is_array="0">
      <description>The command name</description>
      <defaults>
        <default>help</default>
      </defaults>
    </argument>
  </arguments>
  <options>
    <option name="--xml" shortcut="" accept_value="0" is_value_required="0" is_multiple="0">
      <description>To output help as XML</description>
    </option>
    <option name="--help" shortcut="-h" accept_value="0" is_value_required="0" is_multiple="0">
      <description>Display this help message.</description>
    </option>
    <option name="--quiet" shortcut="-q" accept_value="0" is_value_required="0" is_multiple="0">
      <description>Do not output any message.</description>
    </option>
    <option name="--verbose" shortcut="-v" accept_value="0" is_value_required="0" is_multiple="0">
      <description>Increase verbosity of messages.</description>
    </option>
    <option name="--version" shortcut="-V" accept_value="0" is_value_required="0" is_multiple="0">
      <description>Display this application version.</description>
    </option>
    <option name="--ansi" shortcut="" accept_value="0" is_value_required="0" is_multiple="0">
      <description>Force ANSI output.</description>
    </option>
    <option name="--no-ansi" shortcut="" accept_value="0" is_value_required="0" is_multiple="0">
      <description>Disable ANSI output.</description>
    </option>
    <option name="--no-interaction" shortcut="-n" accept_value="0" is_value_required="0" is_multiple="0">
      <description>Do not ask any interactive question.</description>
    </option>
  </options>
</command>
@patcon

This comment has been minimized.

Show comment
Hide comment
@patcon

patcon Sep 12, 2013

Contributor

Seems that maybe Cilex, a more complete cli builder, might be a better route:
https://github.com/Cilex/Cilex

Investigating later, but the main thing is that that config file support is already baked in. Also console-service-providers, which I'm still wrapping my mind around them, seem like they might provide for the extensibility that we've come to expect in Drush...

Contributor

patcon commented Sep 12, 2013

Seems that maybe Cilex, a more complete cli builder, might be a better route:
https://github.com/Cilex/Cilex

Investigating later, but the main thing is that that config file support is already baked in. Also console-service-providers, which I'm still wrapping my mind around them, seem like they might provide for the extensibility that we've come to expect in Drush...

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Sep 12, 2013

Cilex is probably not something you can use as it depends on Pimple, not the Symfony service container (used by Drupal). If there is an interest in moving Drush to the Symfony Console Component, I can help you and it should be painless. We might even talk about that at DrupalCon Prague.

fabpot commented Sep 12, 2013

Cilex is probably not something you can use as it depends on Pimple, not the Symfony service container (used by Drupal). If there is an interest in moving Drush to the Symfony Console Component, I can help you and it should be painless. We might even talk about that at DrupalCon Prague.

@patcon

This comment has been minimized.

Show comment
Hide comment
@patcon

patcon Sep 12, 2013

Contributor

That would be wild, @fabpot :)

Been trying to minimize changes as much as possible by sorting out how to make console apps more "drush-like" (no dups in list for aliases, namespacing without requiring namespace:command format, etc). It would be awesome to speak in-person to get a sense of whether this sort of parity is worth it. Thanks for the offer!

Contributor

patcon commented Sep 12, 2013

That would be wild, @fabpot :)

Been trying to minimize changes as much as possible by sorting out how to make console apps more "drush-like" (no dups in list for aliases, namespacing without requiring namespace:command format, etc). It would be awesome to speak in-person to get a sense of whether this sort of parity is worth it. Thanks for the offer!

@greg-1-anderson

This comment has been minimized.

Show comment
Hide comment
@greg-1-anderson

greg-1-anderson Sep 12, 2013

Member

I'll be at Prague and would love to talk about this. I am all in if it is beneficial and painless.

Member

greg-1-anderson commented Sep 12, 2013

I'll be at Prague and would love to talk about this. I am all in if it is beneficial and painless.

@patcon

This comment has been minimized.

Show comment
Hide comment
@patcon

patcon Sep 12, 2013

Contributor

Just building out what cli could look like. I figure it will be a stupid interface, but someone can steal it when the time comes.

➜  drush git:(console) bin/drush help archive-restore
Usage:
 core:archive-restore [--db-prefix="..."] [--db-su="..."] [--db-su-pw="..."] [--db-url="..."] [--destination="..."] [--overwrite] file [site name]

Aliases: archive-restore, arr
Arguments:
 file                  The site archive file that should be expanded.
 site name             Which site within the archive you want to restore. (default: "all")

Options:
 --db-prefix           An optional table prefix to use during restore.
 --db-su               Account to use when creating the new database. Optional.
 --db-su-pw            Password for the "db-su" account. Optional.
 --db-url              A Drupal 6 style database URL indicating the target for database restore. If not provided, the archived settings.php is used.
 --destination         Specify where the Drupal site should be expanded, including the docroot. Defaults to the current working directory.
 --overwrite           Allow drush to overwrite any files in the destination.
 --help (-h)           Display this help message.
 --quiet (-q)          Do not output any message.
 --verbose (-v|vv|vvv) Increase the verbosity of messages: 1 for normal output, 2 for more verbose output and 3 for debug
 --version (-V)        Display this application version.
 --ansi                Force ANSI output.
 --no-ansi             Disable ANSI output.
 --no-interaction (-n) Do not ask any interactive question.
Contributor

patcon commented Sep 12, 2013

Just building out what cli could look like. I figure it will be a stupid interface, but someone can steal it when the time comes.

➜  drush git:(console) bin/drush help archive-restore
Usage:
 core:archive-restore [--db-prefix="..."] [--db-su="..."] [--db-su-pw="..."] [--db-url="..."] [--destination="..."] [--overwrite] file [site name]

Aliases: archive-restore, arr
Arguments:
 file                  The site archive file that should be expanded.
 site name             Which site within the archive you want to restore. (default: "all")

Options:
 --db-prefix           An optional table prefix to use during restore.
 --db-su               Account to use when creating the new database. Optional.
 --db-su-pw            Password for the "db-su" account. Optional.
 --db-url              A Drupal 6 style database URL indicating the target for database restore. If not provided, the archived settings.php is used.
 --destination         Specify where the Drupal site should be expanded, including the docroot. Defaults to the current working directory.
 --overwrite           Allow drush to overwrite any files in the destination.
 --help (-h)           Display this help message.
 --quiet (-q)          Do not output any message.
 --verbose (-v|vv|vvv) Increase the verbosity of messages: 1 for normal output, 2 for more verbose output and 3 for debug
 --version (-V)        Display this application version.
 --ansi                Force ANSI output.
 --no-ansi             Disable ANSI output.
 --no-interaction (-n) Do not ask any interactive question.
@msonnabaum

This comment has been minimized.

Show comment
Hide comment
@msonnabaum

msonnabaum Sep 12, 2013

Contributor

I personally find the fluent-interface style of command configuration to be too verbose and less expressive than our current hash.

If we want to look at this I think a better approach would be to start using some of the lower level components of Console, and not change the command definition for now.

Contributor

msonnabaum commented Sep 12, 2013

I personally find the fluent-interface style of command configuration to be too verbose and less expressive than our current hash.

If we want to look at this I think a better approach would be to start using some of the lower level components of Console, and not change the command definition for now.

@patcon

This comment has been minimized.

Show comment
Hide comment
@patcon

patcon Sep 12, 2013

Contributor

@msonnabaum I think extending the TextDescriptor is how we would tweak that.

For instance, here's where we'd remove the globally namespaced commands, and remove the namespace prefix on all commands, so that:

Available commands:
  arb                    Backup your code, files, and database into a single file.
  archive-backup         Backup your code, files, and database into a single file.
  archive-dump           Backup your code, files, and database into a single file.
  archive-restore        Expand a site archive into a Drupal web site.
  ard                    Backup your code, files, and database into a single file.
  arr                    Expand a site archive into a Drupal web site.
  help                   Displays help for a command
  list                   Lists commands
  st                     Provides a birds-eye view of the current Drupal installation, if any.
  status                 Provides a birds-eye view of the current Drupal installation, if any.
core
  core:archive-dump      Backup your code, files, and database into a single file.
  core:archive-restore   Expand a site archive into a Drupal web site.
  core:status            Provides a birds-eye view of the current Drupal installation, if any.

Becomes

Available commands:
core
  archive-dump      Backup your code, files, and database into a single file.
  archive-restore   Expand a site archive into a Drupal web site.
  status            Provides a birds-eye view of the current Drupal installation, if any.

Also, drush looks way better in color.

EDIT: Oh wait, this is Drupalier.

Contributor

patcon commented Sep 12, 2013

@msonnabaum I think extending the TextDescriptor is how we would tweak that.

For instance, here's where we'd remove the globally namespaced commands, and remove the namespace prefix on all commands, so that:

Available commands:
  arb                    Backup your code, files, and database into a single file.
  archive-backup         Backup your code, files, and database into a single file.
  archive-dump           Backup your code, files, and database into a single file.
  archive-restore        Expand a site archive into a Drupal web site.
  ard                    Backup your code, files, and database into a single file.
  arr                    Expand a site archive into a Drupal web site.
  help                   Displays help for a command
  list                   Lists commands
  st                     Provides a birds-eye view of the current Drupal installation, if any.
  status                 Provides a birds-eye view of the current Drupal installation, if any.
core
  core:archive-dump      Backup your code, files, and database into a single file.
  core:archive-restore   Expand a site archive into a Drupal web site.
  core:status            Provides a birds-eye view of the current Drupal installation, if any.

Becomes

Available commands:
core
  archive-dump      Backup your code, files, and database into a single file.
  archive-restore   Expand a site archive into a Drupal web site.
  status            Provides a birds-eye view of the current Drupal installation, if any.

Also, drush looks way better in color.

EDIT: Oh wait, this is Drupalier.

@greg-1-anderson

This comment has been minimized.

Show comment
Hide comment
@greg-1-anderson

greg-1-anderson Sep 13, 2013

Member

I think @msonnabaum was referring to how command options are currently defined in code, vs. how they are defined using methods of the Command object. The thing that we want to keep in mind is that Drush is like a scripting language; we want folks to be able to easily pick up, write some simple lines of code to call some Drupal apis, and get out again. If the oop version is significantly more verbose, or adds barriers to beginners, etc., then those are detriments. I think that for shell scripts, brevity is preferable to formality.

I'm not intending to be too critical of unfinished code; I'd just like to focus the discussion on the benefits of using symfony/console, so that we can focus our efforts in the right direction. We had some previous discussion at https://drupal.org/node/1837280

One advantage of symfony is that the object-oriented code would avoid the current potential for namespace collision. Drush, like Drupal, can call the wrong function if you have a commandfile whose name is a substring (beginning of) the name of some other commandfile. Colored help output is also a benefit, although perhaps a slim one. :)

Backwards compatibility is going to be important here, though. There are over 500 projects that implement Drush commands, just counting those that have at least one stable 7.x release. We can't presume that all of these will port to the oop version simultaneously, and it would be a complete non-starter to require a different Drush to interact with different modules in the same Drupal release. If backwards compatibility was impossible, then we could potentially require a different Drush for Drupal 8+; however, this would be a serious detriment, and unlikely to be palatable unless there was strong motivation to adopt symfony/console.

From a UX perspective, we'd also want to avoid stark contrasts in behavior when using the new code. For example, "--no-interaction (-n) Do not ask any interactive question." looks like it behaves like the current --yes / -y Drush option. Currently, -n means --no, which aborts the command if an interactive question is asked. Changing the behavior of -n to be the same as --yes instead of --no would not be acceptable.

It will be interesting to discuss and see if there are good ways that we can use symfony/console that are easy enough to implement, and provide good benefits and improved code for Drush command authors.

Member

greg-1-anderson commented Sep 13, 2013

I think @msonnabaum was referring to how command options are currently defined in code, vs. how they are defined using methods of the Command object. The thing that we want to keep in mind is that Drush is like a scripting language; we want folks to be able to easily pick up, write some simple lines of code to call some Drupal apis, and get out again. If the oop version is significantly more verbose, or adds barriers to beginners, etc., then those are detriments. I think that for shell scripts, brevity is preferable to formality.

I'm not intending to be too critical of unfinished code; I'd just like to focus the discussion on the benefits of using symfony/console, so that we can focus our efforts in the right direction. We had some previous discussion at https://drupal.org/node/1837280

One advantage of symfony is that the object-oriented code would avoid the current potential for namespace collision. Drush, like Drupal, can call the wrong function if you have a commandfile whose name is a substring (beginning of) the name of some other commandfile. Colored help output is also a benefit, although perhaps a slim one. :)

Backwards compatibility is going to be important here, though. There are over 500 projects that implement Drush commands, just counting those that have at least one stable 7.x release. We can't presume that all of these will port to the oop version simultaneously, and it would be a complete non-starter to require a different Drush to interact with different modules in the same Drupal release. If backwards compatibility was impossible, then we could potentially require a different Drush for Drupal 8+; however, this would be a serious detriment, and unlikely to be palatable unless there was strong motivation to adopt symfony/console.

From a UX perspective, we'd also want to avoid stark contrasts in behavior when using the new code. For example, "--no-interaction (-n) Do not ask any interactive question." looks like it behaves like the current --yes / -y Drush option. Currently, -n means --no, which aborts the command if an interactive question is asked. Changing the behavior of -n to be the same as --yes instead of --no would not be acceptable.

It will be interesting to discuss and see if there are good ways that we can use symfony/console that are easy enough to implement, and provide good benefits and improved code for Drush command authors.

@patcon

This comment has been minimized.

Show comment
Hide comment
@patcon

patcon Sep 13, 2013

Contributor

Thanks Greg. I totally agree with the importance of backwards compat. I'm sort of taking it as an article of faith that @fabpot is very aware of Drupal's fondness of all things backward compat, and he seemed confident that using Console was doable :)

In the meantime, I'll mostly be messing around with sorting out how to customize the Console input/output cosmetics to match drush more closely, as that seems more approachable to my skill level for now. Hopefully the "real" work will become clearer after we've spoken to @fabpot in Prague.

Contributor

patcon commented Sep 13, 2013

Thanks Greg. I totally agree with the importance of backwards compat. I'm sort of taking it as an article of faith that @fabpot is very aware of Drupal's fondness of all things backward compat, and he seemed confident that using Console was doable :)

In the meantime, I'll mostly be messing around with sorting out how to customize the Console input/output cosmetics to match drush more closely, as that seems more approachable to my skill level for now. Hopefully the "real" work will become clearer after we've spoken to @fabpot in Prague.

@greg-1-anderson

This comment has been minimized.

Show comment
Hide comment
@greg-1-anderson

greg-1-anderson Sep 13, 2013

Member

Actually, the Drupal project itself is very much of the no-backwards-comparability philosophy. Drush makes API-breaking changes at major releases only (per industry standard), which is about once per year at our current release rate. Drush likes to work with multiple versions of Drupal, though, and the community goes through some serious pain when we make API changes that break important contrib modules (like when the features module didn't work with Drush 5.x for about a year.) I think that we should keep backwards compatibility code for contrib modules in for the previous versions of Drush, so that everyone has a year or so to upgrade before being left behind.

I think that if we made a DrushCommand class that all Drush commands extended, that its execute() method could call drush_invoke (or some new equivalent thereof), which could stitch together the functional command hooks and the oop method command hooks (or maybe make an object wrapper around the existing functional commands?) Then I guess that pre_validate, validate, pre_foo, foo, post_foo and rollback could be methods of the class. I'd also like to have the DrushCommand configure method handle data-driven option initialization; this would be a place where we could merge in the option from any applicable engines.

Member

greg-1-anderson commented Sep 13, 2013

Actually, the Drupal project itself is very much of the no-backwards-comparability philosophy. Drush makes API-breaking changes at major releases only (per industry standard), which is about once per year at our current release rate. Drush likes to work with multiple versions of Drupal, though, and the community goes through some serious pain when we make API changes that break important contrib modules (like when the features module didn't work with Drush 5.x for about a year.) I think that we should keep backwards compatibility code for contrib modules in for the previous versions of Drush, so that everyone has a year or so to upgrade before being left behind.

I think that if we made a DrushCommand class that all Drush commands extended, that its execute() method could call drush_invoke (or some new equivalent thereof), which could stitch together the functional command hooks and the oop method command hooks (or maybe make an object wrapper around the existing functional commands?) Then I guess that pre_validate, validate, pre_foo, foo, post_foo and rollback could be methods of the class. I'd also like to have the DrushCommand configure method handle data-driven option initialization; this would be a place where we could merge in the option from any applicable engines.

@patcon

This comment has been minimized.

Show comment
Hide comment
@patcon

patcon Sep 13, 2013

Contributor

You're totally right. Not sure what I was thinking with that comment :/

👍 on extending a DrushCommand class. I'll start messing around with that approach.

Contributor

patcon commented Sep 13, 2013

You're totally right. Not sure what I was thinking with that comment :/

👍 on extending a DrushCommand class. I'll start messing around with that approach.

@greg-1-anderson

This comment has been minimized.

Show comment
Hide comment
@greg-1-anderson

greg-1-anderson Sep 13, 2013

Member

Another thing you might try. Instead of $cli->add(new StatusCommand);, add a new field to the Drush command record (command-class?), and put "Drupal\Drush\Command\Core\StatusCommand" (& etc.) in it. See drush_parse_command() and drush_command_defaults(). If command-class isn't empty, then you could instantiate a new command of the specified class in drush_get_commands() (so now, the result is going to be DrushCommand objects instead of an array of arrays). When you initialize the command object, you could call the DrushCommand's version of configure() (not sure if you want a polymorphic configure, or a different method) and pass in the $command record. This method could iterate our existing options definitions, and call $this->setName(), $this->addOption(), etc., so folks don't need to change the way they declare their Drush commands. Now you should be able to make a wrapper class (DrushLegacyCommand) that calls the named function hooks for pre_validate() calls and so on. From here, it should be pretty easy to adjust drush_invoke so that it calls methods of the DrushCommand instead of the named function methods. The more tedious change is all of the places in the code that reference a $command object directly will now have to adapt to the fact that $command is an object. (I think it's rare enough for contrib Drush commands to reference $command that we can just let them break in Drush 7 if they do that.)

Extra credit: Take a look at the way that the Drush role commands are implemented. They use class objects, so that we can have different implementations for Drupal 6 and Drupal 7; however, the class loader is kind of wonky. You could make this much better by making the Drush role classes derive from DrushCommand, and make the command handler you wrote (as described above) look for classes named $command['command-class'] . '-' . DRUPAL_MAJOR_VERSION, and instantiate that instead of $command['command-class'] if it exists.

If you do that, then I think that this is backwards compatible + an improvement over the existing code. Win.

I'd still love to brainstorm with you and @fabpot about this & other potential variants and improvements in Prague if you're available. Sometime on Tuesday would probably work best for me.

Member

greg-1-anderson commented Sep 13, 2013

Another thing you might try. Instead of $cli->add(new StatusCommand);, add a new field to the Drush command record (command-class?), and put "Drupal\Drush\Command\Core\StatusCommand" (& etc.) in it. See drush_parse_command() and drush_command_defaults(). If command-class isn't empty, then you could instantiate a new command of the specified class in drush_get_commands() (so now, the result is going to be DrushCommand objects instead of an array of arrays). When you initialize the command object, you could call the DrushCommand's version of configure() (not sure if you want a polymorphic configure, or a different method) and pass in the $command record. This method could iterate our existing options definitions, and call $this->setName(), $this->addOption(), etc., so folks don't need to change the way they declare their Drush commands. Now you should be able to make a wrapper class (DrushLegacyCommand) that calls the named function hooks for pre_validate() calls and so on. From here, it should be pretty easy to adjust drush_invoke so that it calls methods of the DrushCommand instead of the named function methods. The more tedious change is all of the places in the code that reference a $command object directly will now have to adapt to the fact that $command is an object. (I think it's rare enough for contrib Drush commands to reference $command that we can just let them break in Drush 7 if they do that.)

Extra credit: Take a look at the way that the Drush role commands are implemented. They use class objects, so that we can have different implementations for Drupal 6 and Drupal 7; however, the class loader is kind of wonky. You could make this much better by making the Drush role classes derive from DrushCommand, and make the command handler you wrote (as described above) look for classes named $command['command-class'] . '-' . DRUPAL_MAJOR_VERSION, and instantiate that instead of $command['command-class'] if it exists.

If you do that, then I think that this is backwards compatible + an improvement over the existing code. Win.

I'd still love to brainstorm with you and @fabpot about this & other potential variants and improvements in Prague if you're available. Sometime on Tuesday would probably work best for me.

@patcon

This comment has been minimized.

Show comment
Hide comment
@patcon

patcon Sep 17, 2013

Contributor

This is so unbelievably helpful. I could kiss you @greg-1-anderson! Really excited to get more involved in drush dev, as I see it as a perfect testing ground for cool philosophies and approaches that core might one day adopt.

Not sure if I'll get much done before drupalcon, but I'll def be at the sprint -- glad that it's right after my session, so I won't be pre-occupied

EDIT: And Tues works fine by me, although I will likely be a little jittery with preparations.

EDIT2: Feel free to assign this issue to me, if you can. GitHub doesn't let me do that, so sometimes hard to track commitments :/

Contributor

patcon commented Sep 17, 2013

This is so unbelievably helpful. I could kiss you @greg-1-anderson! Really excited to get more involved in drush dev, as I see it as a perfect testing ground for cool philosophies and approaches that core might one day adopt.

Not sure if I'll get much done before drupalcon, but I'll def be at the sprint -- glad that it's right after my session, so I won't be pre-occupied

EDIT: And Tues works fine by me, although I will likely be a little jittery with preparations.

EDIT2: Feel free to assign this issue to me, if you can. GitHub doesn't let me do that, so sometimes hard to track commitments :/

@kostajh

This comment has been minimized.

Show comment
Hide comment
@kostajh

kostajh Oct 2, 2013

Contributor

@patcon @greg-1-anderson At Drupalcon did you all discuss moving forward with this approach?

Contributor

kostajh commented Oct 2, 2013

@patcon @greg-1-anderson At Drupalcon did you all discuss moving forward with this approach?

@greg-1-anderson

This comment has been minimized.

Show comment
Hide comment
@greg-1-anderson

greg-1-anderson Oct 2, 2013

Member

I was in the coder lounge on Tuesday as I suggested, but we hadn't firmly agreed on a schedule. I had network connectivity issues at DrupalCon, and unfortunately missed tweets from @patcon, so in the end, unfortunately, we didn't manage to connect.

Member

greg-1-anderson commented Oct 2, 2013

I was in the coder lounge on Tuesday as I suggested, but we hadn't firmly agreed on a schedule. I had network connectivity issues at DrupalCon, and unfortunately missed tweets from @patcon, so in the end, unfortunately, we didn't manage to connect.

@patcon

This comment has been minimized.

Show comment
Hide comment
@patcon

patcon Oct 3, 2013

Contributor

Yeah, sorry, I dropped the ball as well :( There was just so many things to talk about this drupalcon...!

Contributor

patcon commented Oct 3, 2013

Yeah, sorry, I dropped the ball as well :( There was just so many things to talk about this drupalcon...!

@kostajh

This comment has been minimized.

Show comment
Hide comment
@kostajh

kostajh Oct 3, 2013

Contributor

Ah ok. Well, I'm interested to see what direction this goes in so I'll keep an eye on the issue. I can help out once there is a consensus on what should be done with this.

Contributor

kostajh commented Oct 3, 2013

Ah ok. Well, I'm interested to see what direction this goes in so I'll keep an eye on the issue. I can help out once there is a consensus on what should be done with this.

@greg-1-anderson

This comment has been minimized.

Show comment
Hide comment
@greg-1-anderson

greg-1-anderson Oct 3, 2013

Member

I think my idea would work well, and would give folks the choice of using Drush or Symfony-style command definitions in their Drush commandfiles. I think the next step would be for someone to code up a proposal that implemented just one or two commands, and see how it works.

Member

greg-1-anderson commented Oct 3, 2013

I think my idea would work well, and would give folks the choice of using Drush or Symfony-style command definitions in their Drush commandfiles. I think the next step would be for someone to code up a proposal that implemented just one or two commands, and see how it works.

@jmolivas

This comment has been minimized.

Show comment
Hide comment
@jmolivas

jmolivas Jan 23, 2014

I have been working with another developers on a scaffolding tool for Drupal 8 based on Symfony Console component if you may want to take a look https://github.com/hechoendrupal/DrupalAppConsole

I have been working with another developers on a scaffolding tool for Drupal 8 based on Symfony Console component if you may want to take a look https://github.com/hechoendrupal/DrupalAppConsole

@weitzman

This comment has been minimized.

Show comment
Hide comment
@weitzman

weitzman May 9, 2014

Member

I started looking into how site aliases might fit into a Console powered future and found environments. Seems like a good fit to me. Not sure if we want keep our current @alias syntax or go with --env which is standard for Console apps.

I think we need a wiki page or something where we list out Drush systems and how they would be implemented in a Console based rewrite.

Member

weitzman commented May 9, 2014

I started looking into how site aliases might fit into a Console powered future and found environments. Seems like a good fit to me. Not sure if we want keep our current @alias syntax or go with --env which is standard for Console apps.

I think we need a wiki page or something where we list out Drush systems and how they would be implemented in a Console based rewrite.

@greg-1-anderson

This comment has been minimized.

Show comment
Hide comment
@greg-1-anderson

greg-1-anderson May 9, 2014

Member

@alias is easier to type; perhaps it could be a synonym for --env? Perhaps this is another use case for option aliases.

Another thing that Drush does that is convenient is alias groupings (@site.live and @site.dev, with 'live' and 'dev' defined in the same site alias file). I'm not sure that this feature translates easily, though; perhaps we need to keep each alias in a separate file if we use environments.

Member

greg-1-anderson commented May 9, 2014

@alias is easier to type; perhaps it could be a synonym for --env? Perhaps this is another use case for option aliases.

Another thing that Drush does that is convenient is alias groupings (@site.live and @site.dev, with 'live' and 'dev' defined in the same site alias file). I'm not sure that this feature translates easily, though; perhaps we need to keep each alias in a separate file if we use environments.

@weitzman weitzman added the badcamp label Oct 28, 2014

@barryvdh

This comment has been minimized.

Show comment
Hide comment
@barryvdh

barryvdh Dec 11, 2014

If you don't like the fluent interface etc, you could always create some kind of wrapper around the Command, to do some boilerplate stuff / abstract options/arguments away in your own format.
Laravel does something Similar: https://github.com/laravel/framework/blob/4.2/src/Illuminate/Console/Command.php / http://laravel.com/docs/4.2/commands#building-a-command

With the step from Drupal 8 to integrate more Symfony components, it seems like a logical step to use the Symfony Console for this also.

If you don't like the fluent interface etc, you could always create some kind of wrapper around the Command, to do some boilerplate stuff / abstract options/arguments away in your own format.
Laravel does something Similar: https://github.com/laravel/framework/blob/4.2/src/Illuminate/Console/Command.php / http://laravel.com/docs/4.2/commands#building-a-command

With the step from Drupal 8 to integrate more Symfony components, it seems like a logical step to use the Symfony Console for this also.

@jmolivas

This comment has been minimized.

Show comment
Hide comment
@jmolivas

jmolivas Dec 11, 2014

@barryvdh there is actually a project "Drupal Console" it is an effort to bring the Symfony Console Component to Drupal 8. You can found more information here http://drupalconsole.com/

@barryvdh there is actually a project "Drupal Console" it is an effort to bring the Symfony Console Component to Drupal 8. You can found more information here http://drupalconsole.com/

@barryvdh

This comment has been minimized.

Show comment
Hide comment
@barryvdh

barryvdh Dec 11, 2014

Yes but that's not a replacement for Drush is it?

Yes but that's not a replacement for Drush is it?

@jmolivas

This comment has been minimized.

Show comment
Hide comment
@jmolivas

jmolivas Dec 11, 2014

No is not a replacement it's a separated project and a proof the Console component can be easily integrated with Drupal 8, we already have fully functional commands for Drupal 8, code generators as routing, controllers, forms, plugin block, entities, etc, and another commands interacting with the service container as debugging services and routes, clearing route cache.

No is not a replacement it's a separated project and a proof the Console component can be easily integrated with Drupal 8, we already have fully functional commands for Drupal 8, code generators as routing, controllers, forms, plugin block, entities, etc, and another commands interacting with the service container as debugging services and routes, clearing route cache.

@chx

This comment has been minimized.

Show comment
Hide comment
@chx

chx Dec 11, 2014

Contributor

Deleted all my comments, unsubscribing from this. Nothing I can say can fix anything.

Contributor

chx commented Dec 11, 2014

Deleted all my comments, unsubscribing from this. Nothing I can say can fix anything.

@barryvdh

This comment has been minimized.

Show comment
Hide comment
@barryvdh

barryvdh Dec 11, 2014

Ehhh okay didn't want to start a war or anything.

Ehhh okay didn't want to start a war or anything.

@jmolivas

This comment has been minimized.

Show comment
Hide comment
@jmolivas

jmolivas Dec 11, 2014

Worry not @barryvdh, people have strong opinions, different points of view and everyone is free to express as you did it here and I totally agree

With the step from Drupal 8 to integrate more Symfony components, it seems like a logical step to use the Symfony Console for this also.

Worry not @barryvdh, people have strong opinions, different points of view and everyone is free to express as you did it here and I totally agree

With the step from Drupal 8 to integrate more Symfony components, it seems like a logical step to use the Symfony Console for this also.

@greg-1-anderson

This comment has been minimized.

Show comment
Hide comment
@greg-1-anderson

greg-1-anderson Dec 11, 2014

Member

Drush is really good for some things; for other things, it makes sense to migrate to other tools. I am looking forward to replacing drush make, pm-download and pm-update with composer, for example.

If the introduction of new tools changes the syntax that folks have found convenient and comfortable (e.g. @alias), it might make sense to have drush wrappers that target a site and then call through to the more specific tool. I'm sure that experiments along these lines will happen as time goes along.

Member

greg-1-anderson commented Dec 11, 2014

Drush is really good for some things; for other things, it makes sense to migrate to other tools. I am looking forward to replacing drush make, pm-download and pm-update with composer, for example.

If the introduction of new tools changes the syntax that folks have found convenient and comfortable (e.g. @alias), it might make sense to have drush wrappers that target a site and then call through to the more specific tool. I'm sure that experiments along these lines will happen as time goes along.

@jmolivas

This comment has been minimized.

Show comment
Hide comment
@jmolivas

jmolivas Dec 11, 2014

Taking advantage you already offered to help @fabpot, and based on the comments seems like you already did that on Prage, Do you mind to take a look at https://github.com/hechoendrupal/DrupalAppConsole and let us know if we are in the right direction bringing the Symfony Console Component to Drupal 8.

Taking advantage you already offered to help @fabpot, and based on the comments seems like you already did that on Prage, Do you mind to take a look at https://github.com/hechoendrupal/DrupalAppConsole and let us know if we are in the right direction bringing the Symfony Console Component to Drupal 8.

@jmolivas

This comment has been minimized.

Show comment
Hide comment
@jmolivas

jmolivas Dec 11, 2014

There is no reason to keep maintaining code to achieve tasks other projects are doing properly and also are the standard in the php community, take composer as an example like @greg-1-anderson mentioned.

There is no reason to keep maintaining code to achieve tasks other projects are doing properly and also are the standard in the php community, take composer as an example like @greg-1-anderson mentioned.

@greg-1-anderson

This comment has been minimized.

Show comment
Hide comment
@greg-1-anderson

greg-1-anderson Dec 11, 2014

Member

We are discussing how to use composer with Drush and Drupal sites in #572. Drush dl can do all of the current niceties, like identifying the site you are using, and then call through to composer. For extensions hosted on drupal.org, we could map 'drush dl foo' to 'composer require drupal/foo', and pull version numbers from drupal.org's project xml files to support --select. Similar techniques could be used to provide a Drush-like experience wrapping other commands.

Member

greg-1-anderson commented Dec 11, 2014

We are discussing how to use composer with Drush and Drupal sites in #572. Drush dl can do all of the current niceties, like identifying the site you are using, and then call through to composer. For extensions hosted on drupal.org, we could map 'drush dl foo' to 'composer require drupal/foo', and pull version numbers from drupal.org's project xml files to support --select. Similar techniques could be used to provide a Drush-like experience wrapping other commands.

greg-1-anderson added a commit that referenced this pull request Apr 15, 2015

#88: Include Drupal Console in Drush's composer.json file. Build Drus…
…h command records for all Console commands, so that they can be displayed in Drush's help output, and executed. This allows us to run commands such as 'drush @Remote x-generate-module', and remotely run Console commands with a Drush alias.  Code is still rough at the moment.  Console commands are converted from 'generate:module' to 'x-generate-module' to match Drush command conventions without overlapping with existing Drush commands. This won't be the final transformation, but some discussion will be necessary before we can decide on what it should be.

@greg-1-anderson greg-1-anderson referenced this pull request Apr 15, 2015

Closed

Call Drupal Console commands natively from Drush #1337

3 of 8 tasks complete

@weitzman weitzman added this to the drush9 milestone Mar 17, 2016

@greg-1-anderson

This comment has been minimized.

Show comment
Hide comment
@greg-1-anderson

greg-1-anderson Mar 30, 2016

Member

Closing in favor of #2103. Interested parties following this issue please see that PR.

Member

greg-1-anderson commented Mar 30, 2016

Closing in favor of #2103. Interested parties following this issue please see that PR.

@weitzman weitzman closed this Aug 5, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment