Skip to content

Shell:columns(): Better calc on Windows. Check TERM. Make testable.#105

Merged
danielbachhuber merged 5 commits intowp-cli:masterfrom
gitlost:issue_4127
Jun 8, 2017
Merged

Shell:columns(): Better calc on Windows. Check TERM. Make testable.#105
danielbachhuber merged 5 commits intowp-cli:masterfrom
gitlost:issue_4127

Conversation

@gitlost
Copy link
Contributor

@gitlost gitlost commented Jun 5, 2017

Issue wp-cli/wp-cli#4127

Caters for bash-like shells on Windows that return incorrect columns using mode CON.

Checks that TERM env var exists before using tput otherwise it will write stuff to STDERR, which messes up tests.

Makes testable with env var WP_CLI_TEST_SHELL_COLUMNS_RESET to reset the $columns static.

Uses function_exists() rather than checking disable_functions ini directive as it's more reliable (though to be totally correct should also check suhosin whitelist/blacklist).

Parses output of mode CON in a locale-indifferent way.

@danielbachhuber
Copy link
Member

Checks that TERM env var exists before using tput otherwise it will write stuff to STDERR, which messes up tests.

@gitlost Does this fix https://travis-ci.org/wp-cli/package-command/jobs/239957268 ?

@gitlost
Copy link
Contributor Author

gitlost commented Jun 6, 2017

It would yes, though updating package-command's FeatureContext would also.

@danielbachhuber
Copy link
Member

It would yes, though updating package-command's FeatureContext would also.

Ah, good point. Thanks.

@danielbachhuber danielbachhuber added this to the 0.11.3 milestone Jun 6, 2017
Copy link
Member

@danielbachhuber danielbachhuber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The environment variables should be prefixed with PHP_CLI_TOOLS instead of WP_CLI (as this is a separate, standalone library). Other than that, looks good.

@gitlost
Copy link
Contributor Author

gitlost commented Jun 6, 2017

Ta, that's fixed now.

@danielbachhuber danielbachhuber merged commit 09075e5 into wp-cli:master Jun 8, 2017
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