config-get outputformat mismatch

[weitzman]

I can't seem to figure out how send data to labeled-export outputformat engine. To reproduce the bug, run drush config-get system.site page.front on a D8 site.

[greg-1-anderson]

I am not sure why config-get was defaulting to labeled-export instead of yaml, nor do I know why it was using the dynamic way of setting the default rather than just declaring it in the command record.

The above commit fixes this problem by making the default format yaml. This should be sufficient if all of the formats listed in drush help config-get are sufficient for config-get. If so, go ahead and commit; if you want more formats (like labeled-export), we can take this further -- but then the help listing for config-get would get a lot of extra formats listed, potentially some of which we do not want.

[weitzman]

This seems fine to me. RTBC.

All I can think of that we were trying to match variable-get command, which is the only other place where we use labeled-export. We can probably leave that alone as yaml is unfamiliar to prior versions of Drupal.

[weitzman]

I'm going to use your commit and fix config tests and push.

[weitzman]

OK, I fixed tests and pushed. But I don't think we are perfect here.

1. config-get help has --fields=<field1,field2> but I don't see how these affect anything. What do they mean in the context of yaml?
2. In the case of drush config-get system.site page.front, the key is quoted and the value is not. What I expected, is to only see a value. Thats fine if we keep what we have as default, but I am longing for a string outputformat or other other way to just get that damn value.

[greg-1-anderson]

The patch above allows this:

$ drush config-get system.site page.front --format=string

That gives you the single string that you wanted. We could also choose to call drush_set_default_outputformat('string'); conditionally, whenever we see that the output is a single key/value pair, to make drush automatically print a simple string unless the config item was more complicated.

There is a side effect to the above patch, though; now, all sorts of output formats are offered in drush help config-get. This might be an advantage, if there are instances where some config values may be in formats that the user might want to format with one of Drush's other output formats. If this is not desired, though, I'm not really sure how to allow string, or var_export/yaml/print-r.

The --fields thing is a separate issue. If the output format engine sees that the data type is complex enough that it could have fields, but the command does not define any fields, then it puts in the above generic message to let the user know that they can still filter the output by its keys using --fields, if desired. We could hide this message, but then you'd need to use --strict=0 if you wanted to use --fields in a situation like this.

[weitzman]

You removed one line and all those formats showed up? Some magic there.

Anyway, I think the proposed commit is good. I think we should also:

1. I'm attracted to "automatically print a simple string unless the config item was more complicated". But then we'd want to change vget too and thats an API change and I wonder how worth it all this is...
2. So in the case of yaml and json, --fields is like a way to query and retrieve a section of the document? I can see how that is useful, but we don't advertise the feature very well. I we can't significantly improve the help text for this variant of --fields, we should just hide this option. If the option is hidden rather than removed, then--strict=0 is not needed.

[greg-1-anderson]

The reason that removing the 'output-data-type' adds all of those formats is that by default, commands that do not place any constraints on their engines get all available engines -- which, in the case of output format engines is all output formats. Usually, the output data type should be an array that lists all of the data types that this command can produce. This tells the engine system to select any output format that is compatible with one or more of the output formats that might be emitted. If the output data type is TRUE instead of an array, then the engines are restricted to only those that match an output type, but since no types match, only those output formats that match anything are selected.

Regarding backwards compatibility, my feeling is that since Drupal 8 is still in flux, we could change any D8-specific command like config-get in an API-breaking way, and it wouldn't matter. Of course, we don't officially support D8 in Drush 6, so there would be no need to backport config-get to Drush 6 at all. If you want vget to be consistant with config-get, we could make the change in Drush 7 only. Perhaps we should leave it as it is for a while, and make people use --format=string to force the simple value, and see how it goes. I feel the same way about 'lots of formats'. Maybe there will be some config values where it is useful to format them by some rarely-used formatter. Maybe not. Perhaps we should let it be for a while, and see how it is used.

I think if help alter adds 'hidden' => TRUE to 'fields' if there are no known fields, then it will work the way you want. I can try that. Let me know if any other changes are desired here.

[greg-1-anderson]

Hm, this isn't a pr, so the commit I pushed to the branch w/out referencing this field did not show up automatically. :(

The commit is 01e66ff

I'd tag this 'needs review', if we had a label for it. Do we want that workflow? We could add some more labels. Not sure what our process is going to be. Only maintainers can add labels? Hm.

[weitzman]

I commented on that commit that all looks good except for the minor awkwardness in the phrasing of the code comment.