Skip to content
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

If a Drush command returns a string value, then print it #24

Conversation

greg-1-anderson
Copy link
Member

... even if the command does not define an outputformat record in its command definition.

Previously, a command needed to define an outputformat record in order to get the string to print. e.g.:

'outputformat' => array(
  'default' => 'string',
),

With this patch, this definition may be omitted; however, omitting the outputformat record is not quite the same as including a simple 'string' definition, as shown above. If the outputformat record is included, then the help text for the command will include the option for --format, and it will be possible to format using universal formatters such as var-export and json. Without the outputformat record, the --format option will not appear in the help text, and --format cannot be used with the command.

For simple commands that output only strings, it is probably most appropriate to omit the outputformat record, to keep the command help definition simple.

… command does not define an outputformat record in its command definition.
@weitzman
Copy link
Member

Agreed. Please edit existing commands so they only return their string then feel free to commit to master and 6.x

@greg-1-anderson
Copy link
Member Author

I could not find any commands that needed to be changed for this. Committed to master and Drush 6 branch.

@greg-1-anderson greg-1-anderson deleted the print-string-return-values branch August 22, 2013 23:48
@greg-1-anderson
Copy link
Member Author

Github was confused, because I merged the commit from my branch in my drush repo into drush-ops, rather than checking out the branch that the pull request created in drush-ops. I guess it would have been faster to click the button, and then pull drush-ops. That's odd, though, because that is a commit-then-confirm workflow. I guess I don't know what the merge button does yet; will have to test it next time.

greg-1-anderson added a commit that referenced this pull request Aug 23, 2013
greg-1-anderson added a commit that referenced this pull request Aug 23, 2013
mradcliffe pushed a commit to mradcliffe/drush that referenced this pull request Sep 12, 2013
mradcliffe pushed a commit to mradcliffe/drush that referenced this pull request Sep 12, 2013
mradcliffe pushed a commit to mradcliffe/drush that referenced this pull request Sep 12, 2013
mradcliffe pushed a commit to mradcliffe/drush that referenced this pull request Sep 12, 2013
mradcliffe pushed a commit to mradcliffe/drush that referenced this pull request Sep 12, 2013
mradcliffe pushed a commit to mradcliffe/drush that referenced this pull request Sep 12, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants