Make --pipe awesome with output format engines

'transform-columns' still appears in the comments in the patch above, but ignore that; took it out in favor of the 'filter-input' callback. The purpose here is to allow commands to leave data 'raw' in pipe mode, but fill in human-readable values in 'regular' output modes.

I felt that the output should happen immediately after the main command hook completed, prior to any post-command hooks, so I refactored _drush_invoke_hooks() to take a command array instead of a command hook string. I also re-ordered the function hooks, so that any hooks defined by the primary command file are always called first. This allowed the return values from the main command hooks to be merged together, which would allow us to split up the implementation of commands such as 'drush status' (which would be useful, for example, if we wanted to factor out all of the Drupal-specific parts of Drush into a separate submodule in core).

I also cleaned up the format metadata a bit.

Started to pull in code from #1364808: Proposal for an output formats engine. Attached patch is a bit messy, as it (partially) combines techniques from both patches. Metadata about output formats still defined in the records I added in #2, but these should move to the 'printout' engine. I deliberately omitted the code that allowed for parameterized engine constructors. The existing drush engine code creates an engine object just to call 'validate' on it, and then throws it away; code that uses the engine calls drush_include_engine again to create a new object. In #1364808, the constructed object from drush_include_engine is cached in a context, and code that wants to use it must know to call drush_get_context with a well-named context instead of calling drush_include_engine again, like all existing code does. I also did not like the way that the constructor was always an array of keyed values copied 'by recipe' from the command object (information in the engine info determined which keys are copied into the parameter array). I think that this can probably be improved, and will continue working on it when I next have time. I am posting this in-progress patch just in case someone else wants to look &/or move it forward.

Current status: the output management code from the original patches above still passes the output data to drush_format, but drush_format now uses a printout engine to reformat the data. I defined an 'output-type' field in the printout engines to differentiate between human-readable ("formatted") formats and script-friendly ("pipe") formats; this is currently unused, but can eventually be used to unify the --pipe vs. --format options, so that selecting a pipe-style output format can automatically put Drush into "pipe" mode. The "*-list" output formats implemented above are currently broken; this concept will also need to be implemented in an engine-friendly way.

Put back the caching of the engine instance; it's pretty much the same as it was in #1364808, except I added a couple of trivial wrapper functions and hooked it in to drush_include_engines, so existing code is cached too.

The metadata has for the most part been moved into the engine section of the command, but this is still a work in progress; some sections remain unimplemented. Progress patch for the curious.

Here is a patch that reconciles the concept from #0 with Jonhattan's excellent output engines from #1364808: Proposal for an output formats engine. The biggest difference in this patch from #1364808 is the way that data is formatted before being passed to the output engine. In the original patch, data destined to be output in table format was output as an array of arrays of table cells, which made it inconvenient to convert table-formatted commands into pipe formats (json, var_export, etc.). Now, output is always rendered as key-value arrays, so tables are keyed lists of table rows, each of which is a keyed list of columns. The user-selected field labels are used to order the columns when they are converted to the array-of-array-of-table-cells required by the ConsoleTable library.

Here are some examples of output for a couple of commands:

$ drush version --format=export

array (
  'drush version' => '6.0-dev',
)

Drush version returns an array with a single key/value item. The default format is 'list', which means 'list of strings' by default. The list format outputs the key as the label, and the value in the specified format (string), so the result is:

$ drush version

drush version: 6.0-dev

The default pipe format for Drush version is 'string', which renders the value of the first item of the first array element when passed something more complicated than a string. The resulting output is "6.0-dev".

Also note that the engine is no longer a printing engine, it is a formatting engine. This allows us to unify the --pipe and --format options, so --pipe is just a shorthand for --format=[default pipe format], where the default pipe format can be defined on a per-command basis. This allows us to maintain the current output format for --pipe, while still allowing the user the option of changing the pipe format with --format if desired. (Note that currently, --pipe hides all other output, as it used to, whereas --format does not.)

The core-requirements command is an interesting example. It outputs a key-value array as described above:

$ drush rq --format=export

array (
  'cron' => 
  array (
    'title' => 'Cron maintenance tasks',
    'severity' => 2,
    'value' => 'Last run 1 year 22 weeks ago',
    'description' => 'Cron has not run recently. For more information, see the online handbook entry for <a href="http://drupal.org/cron">configuring cron jobs</a>. You can <a href="/admin/reports/status/run-cron">run cron manually</a>.',
    'severity-label' => 'Error',
  ),
  'drupal' => 
  array (
    'title' => 'Drupal',
    'value' => '6.23-dev',
    'severity' => -1,
    'weight' => -10,
    'severity-label' => 'Info',
  ),
)

This output is truncated (interesting errors from an abandoned dev site not touched for a while :>). The metadata for core-requirements selects the fields to be displayed based on the output format, so the table format has "Info" in the severity field, and the "list" format (used as the default pipe format) has -1.

$ drush rq

 Title                 Severity  Description                                    
 Cron maintenance      Error     Last run 1 year 22 weeks ago                   
 tasks                           Cron has not run recently. For more            
                                 information, see the online handbook entry for 
                                 configuring cron jobs. You can run cron        
                                 manually.                                      
 Drupal                Info      6.23-dev                                       

$ drush rq --pipe

cron: 2
drupal: -1

Another change is in the handling of parameters. The original patch required that the engine info list the field keys from the engine data defined in each command, and this was passed as 'parameters' to the output engine. I have renamed 'parameters' to metadata, and now pass through the union of the engine info 'metadata' field, the engine data defined in each command, and any call-time metadata passed in via code. This allows for the maximum flexibility, so the 'config' engine can use the 'export' output engine (specifying metadata to change the variable name to 'config'), commands can define necessary metadata for their output engines (such as the field label names), and formatting engines can be used in code as well as from command records (so *-list formatting engines can format each line of their output by a user-selectable format engine).

Still needs user documentation, coder documentation, and more commands converted over to the new mechanism. Note that commands that do not define an output engine can still use drush_print_pipe as before. I recommend that we deprecate drush_print_pipe in Drush 6 (and stop using it in Drush core), and remove it in Drush 7. Feedback welcome.

Updated patch with docs for coders and tests.

Updated patch with user documentation (drush topic docs-output-format).

Probably won't have time to work on this for a few days, but I realized in documenting this feature that commands really need a way to be able to specify which types of output formatters they are compatible with -- in other words, they should declare the structure of their output in simple terms. String, set, list, table, or deep array should be a sufficient level of detail. This would also allow us to filter the help output to remove those output formatters that were not relevant to the command. In conjunction with this, to give the *-list formatters a place to 'hang their hat' in the help output, I will also be removing the regex-based engine selection feature, and declaring each formatter individually in the engine info list.

Here is an updated patch that fixes up the help text for commands that use outputformat engines. Now, only output formats that are compatible (or at least likely to be compatible) with the command are shown in help. Also, the default pipe format is also shown.

This patch fixes a minor typo in #10 that broke pm-info.

I am still very much attached to --pipe, as it is much faster to type than --format= (pause to scratch your head as you try to remember which format looks best for this particular command). I'm open to discussion on dropping it, depending on others' opinions, though.

Regarding sample commands, formatted output is only interesting and useful for commands that produce structured output -- anything that outputs in a list or a table is likely to be good. Since the sandwich example produces a rendered picture that is not readily translatable into json or ini formats, it would not make a good example. Similarly, the xkcd example opens a browser window, and is therefore also sub-optimal for demonstrating formatting concepts.

Perhaps what is needed is a new Drush example commandfile that defines three or so sample commands: one that produces simple output like 'drush version', one that produces a key-value list, like 'drush status', and one that produces a deep array, like 'drush pm-info'. I'll do that post-review, after any needed API changes are made.

I just remembered something I was going to address, but forgot about: column-widths should not be based on column number; it needs to be translated from the column label. Otherwise, if you change the order of the columns with the --fields option, the column widths will get messed up.

Thank you for the review and agreement.

Since the patch is considerably huge, I'm for committing this almost as

That is fine by me. At the moment (until we change something), the current code is backwards-compatible with existing handling, so it wouldn't hurt anything to commit an early version, if that is what is desired.

just drop it (--pipe) in 6.x and document it in a change record.

Fine by me. That will need to happen after all core commands are converted first, of course.

we should do something to avoid passing both pipe and format in the command line

Agreed. Not addressed yet.

json should be eligible for format-associative-arrays

json was accidentally tagged as requiring deep arrays only. Fixed.

This should be a two items array?

Yep; typo - fixed.

Why not binary constants and bitwise OR operations instead of arrays?

The main reason I did it this way is that the feature is defined as a generic feature of the Drush engines system, and it seemed easier and more extensible to use lists of string constants rather than defined bitmaps. If someone started providing new engine implementations in contrib, and their new implementation defined a new capability, then they would need to pick a new bitmap value, which could conflict with some other contrib extension that unknowingly did the same thing. This is probably not an issue with outputformat engines, since I can't think of any new 'capabilities' (format types) right now, but in the generic, there might be some.

underscore is missing?

Yes, typo; fixed.

--pipe and --format=string are not equivalent. The diff is a \n

This should resolve itself once the drush_print_pipe mechanism is removed. Note that if we do remove drush_print_pipe, it means that log messages will print to stderr in --pipe mode, whereas previously they did not. I think this is a good thing, but we can discuss. We'll also need to decide which way to resolve the \n issue.

The output data could not be processed by the selected format 'string': array provided where a string is required.

This is primarily a confusion with error reporting; it is actually behaving "correctly", or at least "as implemented". By default, --list == "list of strings", whereas the default format for Drush status is "key-value". The "key-value" format is very similar to "list", but, in addition to rendering the label for each list item, it will also expand values that are arrays into space-separated lists of items. There are a couple shortcomings in the implementation that make the above error message confusing. There is no way to represent the difference between the capabilities of 'list' vs. the capabilities of 'key-value' vis-a-vis their ability to expand simple arrays inside their values. Both claim to have the capability 'format-associative-arrays', but 'key-value' goes a little farther (although not all the way to the general 'format-deep-arrays'). Also, when list does its rendering, it does so with a recursive call to the format it is using to render the elements. Therefore, the error message says that the selected format is 'string', whereas the user selected the format 'list'. It might be better to pass extra info in to the subformat, so that the error message can read 'list of strings' instead of just 'string', or something like that. Other suggestions welcome.

Need to convert whitespace to underscore in variable names.

Agreed.

Also, the 'fields' feature still needs some love. It is the responsibility of each output format to handle requests for specific fields, since the meaning of --fields varies depending on the format of the data. More work needs to go into the different formatters to make this behavior consistent.

Regarding my comments on sample commands in #13, I think that the right thing to do here is to add a couple of new commands to the 'sandwich' commandfile; something like 'sandwiches-served' and 'spreads-status', etc. I'll work on that later.

Here's a new version of the patch with just a couple of trivial typos (noted above) fixed, + rebased on the latest 8.x-6.x branch.

1. The formatting in help here is done per the standard help processing for engines. We could change this, but please take a look at drush help pm-download side-by-side with the above, and note how options specific to an engine instance are grouped with the engine instance's help line. In the case of --version-control engines and others, where each engine instance has a bunch of options, the existing format looks good. In the case of output formats, where none of the formats have specific sub-options yet, it looks like the help text could be consolidated. If we tried to do this in a generic way (consolidate only if there are sub-options), then we'd have to consider what happens after someone adds a sub-option to a new format. I suppose this is an issue even if we special-case output format help - what happens to format type sub-options, should any be implemented? Perhaps we should defer a decision on how to format help until everything else is done, to see how important sub-options for format types is.

4. Permanently changing existing 'severity' to 'severity-id' works for me.

2. & 9. What if we got rid of the top-level 'engines' item in command structures for all commands? Drush already knows the name of the valid engines from the drush_engine_type_info() hook. The danger here is that if contrib created a new engine type, then its name must not conflict with any other top-level item in a command record. This could probably be solved by convention - requiring a prefix for contrib engine names, for example.

6. Yes, there are a bunch more commands that should adopt this once the details are worked out. Commands can either declare an outputformat engine, as shown here, or drush_format_output() may be called directly if necessary.

10. I don't see any way to enforce adoption in code. Commands that do operations and produce incremental output will always need to be able to output directly to the screen.

12. I inherited drush_engine_type_help_alter() from Jonhattan's earlier effort. I can look into whether this could be simplified, but it would be nifty if he already knew the answer and could provide it. :)

14. Backend responses can't be an output format engine; they behave quite a bit differently, and put more than just the output from the command into the result. It might be possible to shoe-horn it, but I'm a little skeptical. Note also that not all commands will use output format engines, but all commands must work with --backend.

15. I definitely wanted functional tests, that made sure that the option handling & c. from drush were working correctly, etc.; however, I admit that my functional tests look like unit tests. Perhaps this could be improved later.

16. Space-delimited in a cli option is a little unusual (--fields="a b c"), but perhaps we could use the code from pm-info & c., and allow both space and comma-delimited inputs.

I have already started to add structured command examples to mmas; I think it would be awesome if you could also add some to the xkcd command. You might want to wait for #18.9 to land, if we do in fact decide to do that, although rework probably wouldn't be hard if you did it sooner.

1. Maybe add 'hidden' to engine instances, just as command options already have, and modify --help to show hidden stuff in verbose mode? There are a lot of hidden global options that could use some exposure in this way, too.

17. Maybe we should use key-value as the default format for core-requirements, and remove label processing from list. This would make the behavior of list more consistent with its name, and give it better separation from the key-value format.

I wish there was a better name for --key-value-list.

I think that a) is what Moshe was suggesting (c.f. #22), but I haven't yet looked at how convoluted the implementation would be. It seems fair that drush_COMMANDFILE_help_alter would be responsible for doing help alterations for every engine it adds.

As for b), I don't think I'm in favor of making outputformat its own new kind of plugin; however, I think we could potentially simplify the way that engine data is expressed in command definitions (e.g. #19 2 & 9).

Seeing the code quoted in #24 brings to mind another point: append-columns should be called concatenate-columns, and perhaps should be a format-specific option.

Progress patch. Implemented sample commands that use formatters in the example sandwich commandfile. Most of the minor points raised above are addressed, but some larger issues still tbd, as described below.

Still to do:

Need to make --fields implementation ubiquitous (in formatters where it is relevant).

Need to make variable names php-friendly.

Need to implement formatters for all the other core commands that use drush_print_pipe.

Have not touched help_alter yet.

Guidance needed:

(1) What is the best resolution for --format= entries in help? Combine engine help items into a single line if there are no sub-options, and break them all out if one or more has sub-options?

(4) I started to implement the switch from 'severity' to 'severity-id'; the problem is that Drupal returns a data structure that contains 'severity', so it seemed wrong to rename this to 'severity-id'. I still wanted users to be able to say --fields=severity instead of severity-label, though. Thoughts? Maybe this is not important, and we should just remove "the dance" and leave this as-is.

(9) Should we remove 'engines' nesting for all command engine data? I don't think we should special-case formatters, but it would be okay if we did them all. Engine names would have to carefully not conflict with other top-level command data keys.

Here's an interdiff for #26.

One more thing I forgot to mention about #26. I did not remove the spaces in field name help per (16). The problem is that this list can get really long, and help formatting gets pushed out if the spaces are removed. I adjusted the code so that it works with or without the spaces after the commas. This behavior is now standard in drush_get_option_list().

I forgot to answer #17 (7). format_error() does not take an error code, as it invariantly uses DRUSH_FORMAT_ERROR. The purpose for having error codes is so that code that calls Drush can detect and react to specific kinds of errors individually. Since DRUSH_FORMAT_ERROR is usually only encountered if you request the wrong kind of format for the command being called, it typically won't happen unless there is a coding error. It therefore does not make sense to individually check for a specific kind of format error, because you can instead fix the code to avoid the format error. In the rare cases where code may encounter a format error, I think it's actually more useful to be able to test for "any format error" rather than list every specific one that you expect you might get.

Update to fix some things #26 broke in core-requirements.

#1862950: drush site-alias should indicate the current alias set by drush use will need to be redone once this patch lands.

Progress patch.

- Remove engine-specific help alter, and just use existing commandfile help alter
- Fix output format test after changing behavior of 'list' output formatter.
- Filter invalid characters out of variable names in export output formatter.
- Make commands with single-value results, such as 'version' and 'sandwiches-served' return a single value

c.f. #26

I decided that it was okay to remap the 'severity' field in core-requirements. I remapped it to 'sid' instead of 'severity-id', though.

I also switched the output format of core-requirements from 'list' to 'ini'. List used to output each line as "key: value", which was very similar to key-value, which outputs lines as " key : value" (aligned). However, I changed 'list' to output each line as just "value" to better match users' expectations of what a list should look like. 'key-value' would have been a closer match to the old output of core-requirements --pipe, but 'key-value' is a formatted output, not a pipe output,and it didn't seem worthwhile to add a new output format just to maintain the "key: value" output style.

Progress patch.

Formats are now all grouped into one line. Rather than key this behavior off of the occurance of suboptions, engines declare that they want their help to all appear on one line via the 'combined-help' item. Currently, the help contains the full description of each engine instance, but this should be shortened just to a list of available options, per #34. Help output for commands with other sorts of options (e.g. drush help pm-download) is unchanged. Suboptions might go under the main engine option, or it might take some work before it does this; output formats do not have suboptions, and I have not tried putting any in yet. Perhaps this should not be implemented until needed.

The starting of an output format topic is included; it puts all of the engine instances help at the end of the file. (More work on the text that appears here is needed.) Example commands still exist in the documentation, but they are hardcoded into the topic file. Ideally, the example commands would also be dynamically generated from data present in the 'engines' section of each command that wanted to be included in the topic text. Once command examples are dynamically generated, they can be placed at the end of the topic text, after all of the engine instance help text. Note that currently, this code is implemented as an outputformat-specific topic command; however, the code is already fairly generic, and I think it could become generic engine code that any engine type could use to declare a topic command that dynamically collects the text to display from the different engine instances. This would probably go in core.drush.inc, since engines.inc is not a commandfile.

Can they really be merged? 'combine-help' still adds the options to the command -- it just combines them into a single option. Perhaps you were suggesting re-doing this option so that it selects one of three styles: 'engine-option-help' => 'combined', 'individual' or 'off', perhaps?

#38 is a good idea.

If you get 'unknown engine type' errors as shown in #38, the problem is that outputformat.drush.inc is not in your commandfile cache. Strangely, drush cc drush did not do it for me; I did something like rm -rf /tmp/drush*. I would think that clearing the Drush command cache should have worked, but I didn't really investigate.

Sure, #40 is doable. @Moshe, do you have an opinion on which way it should go? If I implemented #40, then 'engine-option-help' would be the default (c.f. version control engine help output, as shown in pm-download help), some engines would specify 'engine-option-help' => 'combined' (e.g. the output format engine), and some engines would specify 'engine-option-help' => 'none' or 'off' (any engine that has 'add-options-to-command' => FALSE today). Command authors would not be affected. Implementation pretty trivial; don't care too much which way it goes myself. One switch is better than two, but on the flip side, boolean options are easier than arbitrary constants.

I think I'll leave it as it is unless @jonhattan feels strongly about his suggestion.

Here's a version that provides a generic implementation for engine topics; any engine can define a topic command, and it will appear in the Drush topic list. Output is taken from the provided topic file, plus info collected from all of the engine instances (in our case, these are the individual output formats).

The text that appears in the help could use some work; I think I may just remove the command examples and incorporate them into the help text for the different output formats (so each shows some code + the formatted output, and some might mention a Drush command that the format is useful to use with).

I think with docs cleanup + any comments on this version of the patch, that perhaps we could commit at this stage and work on porting remaining Drush commands that need to implement this feature in follow-up issues.

Okay. I am still working through the documentation changes I mentioned above, as time permits; it is taking longer than I thought it would. I'll probably be ready to commit in a couple more days, though.

Why do we need the concept of "engines" to be in the command definition?

The initial patch here was done as you suggest, but was reworked to merge in the engines concept from #1364808: Proposal for an output formats engine. Engines are how Drush does command-related plugins. Output formatters are command-related plugins; therefore, I think they should be engines.

Why not just have "outputformats" as a key?

There was some discussion about this in #26:

(9) Should we remove 'engines' nesting for all command engine data? I don't think we should special-case formatters, but it would be okay if we did them all. Engine names would have to carefully not conflict with other top-level command data keys.

I am not opposed to revisiting how engine data is represented in commands, but changes in this area should be consistent with all of the existing engines. It would be a lot of work, though; maybe this should be a follow-on issue?

Do we want configurable "pipe" formats? Why not just have a CSV export type and alias --pipe to it?

Existing Drush commands are not consistent in the format they choose to use to present their pipe data; the current patch allows commands to maintain the same pipe output that existed before. It would be possible to unify --pipe, so that it always produced a consistent output type regardless of the command. However, CSV wouldn't be a good choice for this, as not every command's pipe output can be represented in a CSV list. We could use JSON or YAML though.

Are there reasonable use cases for a command to return multiple data types? If not, it could greatly simplifies the command definition.

Not sure what you are asking here. Ideally, every command returns a data type that is always consistent in its structure, and the selected formatter transforms the output as necessary. Perhaps this is related to the 'require-engine-capability', below.

The 'concatenate-columns' option seems like more flexibility than we need.

The core requirements command puts two fields in one table cell to match the way that the output looks in the web interface. Perhaps this could be implemented as an output filter specific to core requirements that merges the columns together prior to passing them to the output formatter.

The functionality of "require-engine-capability" seems reasonable, but the name is very confusing and also exposes the word "engine" again. Aren't you just specifying what data types your command returns?

I suppose this needs more work. Naming ideas are welcome. The name includes "engine" because this feature is implemented in the generic engine code, to allow the set of eligible engines to be reduced based on the needs of the command. For output formatters (the only engine that use the feature), this does boil down to the output format of the command.

I'm very unclear what the difference would be between format-associative-arrays and format-deep-arrays, and I dont see any explanation in the patch.

I'm working on documenting this in the topic command, but I'm not done with it yet. Basically, format-associative-arrays indicates that the output is an array of key/value pairs with simple string values. format-deep-arrays indicates that the output is an associative array where the value of each item is itself an associative array. An example of this is the table type. I think that better names would be helpful here; it would also be helpful if there was a stricter definition of what each of these capabilities mean. Some formatters can process a key/value (string) array, or they can take a deep array and convert the inner arrays into a simple string value, either by converting an array into a csv, or perhaps by extracting a single named element from it. Maybe these formatters need to be identified by a third "capability". I'm not sure what is best; suggestions are welcome.

I've just picked up some additional commitments, so someone else might need to move this forward if a lot of rework is desired, like changing the way engines work in Drush, or going back to the implementation prior to the patch in #4. It might be easier to make progress with incremental patches after the initial commit, if we can get it to a level that folks can live with. If there is some agreement on these points, I'll try to fix up the other issues that were raised.

I can pick this up again in a month or three.

I addressed a few of the issues in #47.

I simplified the way that commands declare their output compatibility with the various output format engines. Now there is just 'format-single', 'format-list' (formerly 'format-simple-lists' and 'format-associative-arrays') and 'format-table' (formerly 'format-deep-arrays'). Commands now declare just one of these output types; format engines might declare that they support multiple of these, if they do in fact understand multiple formats. I also removed the feature that switched the output format when a 'list' type returned a single element.

I renamed 'output-type' => 'pipe' to 'machine-parsable' => TRUE.

I also made some minor adjustments to insure that all the unit tests passed, added the docs-output-formats topic, and committed as 27cbbaf. Further work can continue in follow-on issues.

Follow-on issues:

#1912616: Simplify the declaration of engine data in commands.
#1912628: Adapt other core Drush commands to output engines
#1912630: Add a yaml output format
#1912634: Support --fields in drush core-status

If there are any unresolved issues above that still need discussion, please open a new issue for it.