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

Improve validation for --regex-flags and --regex-delimiter and add tests for them #30

Merged
merged 8 commits into from Aug 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 20 additions & 1 deletion features/search-replace.feature
Expand Up @@ -174,6 +174,14 @@ Feature: Do global search/replace
Scenario: Regex search/replace with a incorrect `--regex-flags`
Given a WP install
When I try `wp search-replace '(Hello)\s(world)' '$2, $1' --regex --regex-flags='kppr'`
Then STDERR should contain:
"""
(Hello)\s(world)
"""
And STDERR should contain:
"""
kppr
"""
And the return code should be 1

Scenario: Search and replace within theme mods
Expand Down Expand Up @@ -355,10 +363,21 @@ Feature: Do global search/replace
http://example.jp
"""

When I run `wp search-replace 'http://example.jp/' 'http://example.com/' wp_options --regex-delimiter='/'`
Then STDOUT should be a table containing rows:
| Table | Column | Replacements | Type |
| wp_options | option_value | 2 | PHP |

When I run `wp option get home`
Then STDOUT should be:
"""
http://example.com
"""

When I try `wp search-replace 'HTTP://EXAMPLE.COM' 'http://example.jp/' wp_options --regex --regex-flags=i --regex-delimiter='1'`
Then STDERR should be:
"""
Error: Incorrect regex delimiter.
Error: The regex '1HTTP://EXAMPLE.COM1i' fails.
"""
And the return code should be 1

Expand Down
24 changes: 13 additions & 11 deletions src/Search_Replace_Command.php
Expand Up @@ -91,7 +91,7 @@ class Search_Replace_Command extends WP_CLI_Command {
* : Pass PCRE modifiers to regex search-replace (e.g. 'i' for case-insensitivity).
*
* [--regex-delimiter=<regex-delimiter>]
* : The delimiter to use for the regex. It must be escaped if it appears in the search string.
* : The delimiter to use for the regex. It must be escaped if it appears in the search string. The default value is the result of `chr(1)`.
*
* [--format=<format>]
* : Render output in a particular format.
Expand Down Expand Up @@ -139,18 +139,20 @@ public function __invoke( $args, $assoc_args ) {
$this->verbose = \WP_CLI\Utils\get_flag_value( $assoc_args, 'verbose' );
$this->regex = \WP_CLI\Utils\get_flag_value( $assoc_args, 'regex' );
$this->regex_flags = \WP_CLI\Utils\get_flag_value( $assoc_args, 'regex-flags' );
$this->regex_delimiter = \WP_CLI\Utils\get_flag_value( $assoc_args, 'regex-delimiter', '/' );
$this->regex_delimiter = \WP_CLI\Utils\get_flag_value( $assoc_args, 'regex-delimiter', chr( 1 ) );
$this->format = \WP_CLI\Utils\get_flag_value( $assoc_args, 'format' );

// http://php.net/manual/en/reference.pcre.pattern.modifiers.php
if ( $this->regex_flags && ! preg_match( '/^(?!.*(.).*\1)[imsxeADSUXJu]+$/', $this->regex_flags ) ) {
WP_CLI::error( "Incorrect PCRE modifiers." );
}

if ( empty( $this->regex_delimiter ) ) {
$this->regex_delimiter = '/';
} elseif( ! preg_match( '/^[^0-9\\s]{1}$/', $this->regex_delimiter ) ) {
WP_CLI::error( "Incorrect regex delimiter." );
if ( ! empty( $this->regex ) ) {
if ( '' === $this->regex_delimiter ) {
$this->regex_delimiter = chr( 1 );
}
$search_regex = $this->regex_delimiter;
$search_regex .= $old;
$search_regex .= $this->regex_delimiter;
$search_regex .= $this->regex_flags;
if ( false === @preg_match( $search_regex, '' ) ) {
\WP_CLI::error( "The regex '$search_regex' fails." );
}
}

$this->skip_columns = explode( ',', \WP_CLI\Utils\get_flag_value( $assoc_args, 'skip-columns' ) );
Expand Down
6 changes: 5 additions & 1 deletion src/WP_CLI/SearchReplacer.php
Expand Up @@ -84,7 +84,11 @@ private function _run( $data, $serialised, $recursion_level = 0, $visited_data =

else if ( is_string( $data ) ) {
if ( $this->regex ) {
$data = preg_replace( "$this->regex_delimiter$this->from$this->regex_delimiter$this->regex_flags", $this->to, $data );
$search_regex = $this->regex_delimiter;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you move this check to the top level where you had the previous check so it only gets run once?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is moved:

if ( ! empty( $this->regex ) ) {
$search_regex = $this->regex_delimiter;
$search_regex .= $old;
$search_regex .= $this->regex_delimiter;
$search_regex .= $this->regex_flags;
if ( false === @preg_match( $search_regex, '' ) ) {
\WP_CLI::error( "The regex '$search_regex' fails." );
}
}

$search_regex .= $this->from;
$search_regex .= $this->regex_delimiter;
$search_regex .= $this->regex_flags;
$data = preg_replace( $search_regex, $this->to, $data );
} else {
$data = str_replace( $this->from, $this->to, $data );
}
Expand Down