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

Annotated command implementation of 'core-status' command. #2437

Merged
merged 7 commits into from
Nov 19, 2016

Conversation

greg-1-anderson
Copy link
Member

The handling of default fields is not working right. If --fields is not specified, an error is returned. If a valid list of --fields is given, they will be printed.

@greg-1-anderson
Copy link
Member Author

Fixed one bug in the upstream, but one more remains. All of the possible fields are displayed, even if some fields contain no data. Fields with no data should be omitted from the output.

@weitzman
Copy link
Member

weitzman commented Nov 8, 2016

@greg-1-anderson - Does your recent output-formatters work fix #130 for annotated commands?

@greg-1-anderson
Copy link
Member Author

@weitzman Yes, with the addition of the --include-field-labels option to the status command. This could be a global option, perhaps.

Having a hard time figuring out why the tests are failing. Sometimes the site-under-test ends up at drush-sandbox/web/web instead of drush-sandbox/web. It seems that maybe archive-dump or archive-restore or something else in the tests is relying on the behavior of drush core-status that I am not completely emulating. This is not a great design. Might want to fix it by changing the way the tests work.

@greg-1-anderson
Copy link
Member Author

greg-1-anderson commented Nov 8, 2016

Okay, got the tests using only "standard" forms, no shortcuts, when using core-status.

Not implemented yet:

  • --format=tsv blows up (does not handle non-string cell data)
  • --format=csv not working perfectly (again, array handling a bit off)
  • Using arguments in place of --fields=... not implemented (drop support for this?)
  • --pipe selects tsv; should select json to maintain backwards compatibility

@weitzman
Copy link
Member

weitzman commented Nov 8, 2016 via email

@greg-1-anderson greg-1-anderson changed the title Preliminary implementation of the 'status' command. Annotated command implementation of 'core-status' command. Nov 9, 2016
@weitzman
Copy link
Member

@greg-1-anderson - I'm OK if we merge this without all that checklist complete.

@greg-1-anderson
Copy link
Member Author

Latest commit should take care of the primary backwards-compatibility issues. Let's see how the tests fare.

@greg-1-anderson greg-1-anderson merged commit a557cf4 into master Nov 19, 2016
@weitzman weitzman deleted the status-command branch November 26, 2016 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants