Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 6, 2025

Add --search-replace option to vip dev-env sync sql command

Summary

This PR adds a --search-replace (alias -r) option to the vip dev-env sync sql CLI command that performs search/replace operations on the SQL file during sync, matching the approach used in vip dev-env import sql.

Implementation Approach

Instead of running WP-CLI search-replace after database import, this implementation:

  1. Adds user-provided search-replace pairs to the existing searchReplaceMap
  2. Uses the existing runSearchReplace() method which operates on the SQL file using @automattic/vip-search-replace
  3. This is the same approach used by dev-env-import-sql via resolveImportPath()

Changes Made

CLI (src/bin/vip-dev-env-sync-sql.js)

  • Added --search-replace / -r option with array normalization
  • Passes normalized pairs to DevEnvSyncSQLCommand constructor
  • Added usage examples

Command (src/commands/dev-env-sync-sql.ts)

  • Added userSearchReplacePairs readonly property to constructor
  • addUserSearchReplacePairs(): Parses "search,replace" strings, validates, and adds to searchReplaceMap
  • Updated generateSearchReplaceMap() to call addUserSearchReplacePairs() and fixed early return bug
  • User pairs are added on top of automatic domain replacements

Tests (__tests__/commands/dev-env-sync-sql.ts)

  • 5 new tests covering parsing, validation, comma handling, and error cases

Benefits

  • More efficient: operates on SQL file before import (no WP-CLI overhead)
  • Consistent with existing import-sql behavior
  • Reuses existing, well-tested search-replace infrastructure
  • No additional database operations needed

Usage

# Single operation
vip @app.env dev-env sync sql --slug=site -r "old.com,new.com"

# Multiple operations
vip @app.env dev-env sync sql --slug=site -r "old,new" -r "foo,bar"

# Replace value can contain commas
vip @app.env dev-env sync sql --slug=site -r "old,new,value"
Original prompt

Problem summary:
Add a --search-replace (alias -r) option to the vip dev-env sync sql CLI command and implement the functionality to perform WP-CLI search/replace operations after the database import completes locally. This should match the UX and normalization implemented in vip dev-env import sql (support multiple instances or single value normalized to an array). The implementation must be secure (avoid shell interpolation), performant, and follow Clean Code principles. It should integrate with existing tracker usage.

Files to change:

  1. src/bin/vip-dev-env-sync-sql.js
  • Add a new option definition for [ 'r', 'search-replace' ] with description mirroring import-sql.
  • Normalize opt.searchReplace to an array (if provided) before constructing the DevEnvSyncSQLCommand, consistent with src/bin/vip-dev-env-import-sql.js.
  • Pass searchReplace through to the DevEnvSyncSQLCommand via an extraOptions object.
  1. src/commands/dev-env-sync-sql.js
  • Implement parsing/validation helper(s) for the search-replace pairs (e.g., normalizeSearchReplace) that accept values like ["old,new", "foo,bar"] and return an array of objects { search, replace }.
  • After the database import step (post-import), if search/replace pairs are provided, for each pair run WP-CLI search-replace inside the local environment using a safe execution method (spawn with args array, no shell interpolation).
  • Use safe WP-CLI flags: --all-tables, --precise, --skip-columns=guid, --quiet. Add --network when env.isMultisite is true.
  • Ensure the implementation integrates with trackerFn to log start/done events for the search-replace operation.
  • The developer may already have helpers for executing WP in Lando. If so, prefer them; otherwise implement a secure fallback using child_process.spawn.
  • Keep helpers small and testable: normalizeSearchReplace, buildSearchReplaceBaseArgs, execWpCliInLando.

Implementation details & constraints:

  • Behavior must match import-sql for option parsing: if a user passes a single --search-replace value it becomes a single-element array; multiple occurrences become an array.
  • Accept comma inside values only as delimiter between search and replace; only split on the first comma so replace strings may include commas. Trim whitespace.
  • Validate that a search value is non-empty; allow replace to be empty.
  • Use spawn with shell: false and pass arguments as arrays to avoid shell injection.
  • Add tracker events 'search_replace_start' and 'search_replace_done' around the operation.
  • Keep the existing command cancellation semantics (if existing run() returned false when canceled, preserve that behavior).

Testing considerations:

  • Confirm CLI accepts --search-replace and normalizes values in both single and multiple forms.
  • Confirm that when searchReplace is provided the command runs WP-CLI search-replace with the expected flags and succeeds when WP-CLI returns 0.
  • Confirm that multisite adds --network flag.

List of concrete changes to create in the PR (high level):

  • Modify src/bin/vip-dev-env-sync-sql.js to add CLI option and pass option through.
  • Add/modify src/commands/dev-env-sync-sql.js to implement logic described above. If the file already exists, update it; otherwise create it.

Please create a branch, make the changes, and open a pull request with a concise title and description summarizing the changes and rationale. Include notes in the PR description suggesting that if a project helper exists to run WP inside Lando, execWpCliInLando should be replaced with that helper for consistency.

Files/content to update (high-level - the agent should produce actual edits):

  • src/bin/vip-dev-env-sync-sql.js: add option block for [ 'r', 'search-replace' ], normalize opt.searchReplace to array, pass into DevEnvSyncSQLCommand as extraOptions.
  • src/commands/dev-env-sync-sql.js: implement the search-replace logic and helper functions as described.

Make the changes with clean, secure code and include basic inline comments where needed.

Do not modify unrelated files.

This pull request was created as a result of the following prompt from Copilot chat.

Problem summary:
Add a --search-replace (alias -r) option to the vip dev-env sync sql CLI command and implement the functionality to perform WP-CLI search/replace operations after the database import completes locally. This should match the UX and normalization implemented in vip dev-env import sql (support multiple instances or single value normalized to an array). The implementation must be secure (avoid shell interpolation), performant, and follow Clean Code principles. It should integrate with existing tracker usage.

Files to change:

  1. src/bin/vip-dev-env-sync-sql.js
  • Add a new option definition for [ 'r', 'search-replace' ] with description mirroring import-sql.
  • Normalize opt.searchReplace to an array (if provided) before constructing the DevEnvSyncSQLCommand, consistent with src/bin/vip-dev-env-import-sql.js.
  • Pass searchReplace through to the DevEnvSyncSQLCommand via an extraOptions object.
  1. src/commands/dev-env-sync-sql.js
  • Implement parsing/validation helper(s) for the search-replace pairs (e.g., normalizeSearchReplace) that accept values like ["old,new", "foo,bar"] and return an array of objects { search, replace }.
  • After the database import step (post-import), if search/replace pairs are provided, for each pair run WP-CLI search-replace inside the local environment using a safe execution method (spawn with args array, no shell interpolation).
  • Use safe WP-CLI flags: --all-tables, --precise, --skip-columns=guid, --quiet. Add --network when env.isMultisite is true.
  • Ensure the implementation integrates with trackerFn to log start/done events for the search-replace operation.
  • The developer may already have helpers for executing WP in Lando. If so, prefer them; otherwise implement a secure fallback using child_process.spawn.
  • Keep helpers small and testable: normalizeSearchReplace, buildSearchReplaceBaseArgs, execWpCliInLando.

Implementation details & constraints:

  • Behavior must match import-sql for option parsing: if a user passes a single --search-replace value it becomes a single-element array; multiple occurrences become an array.
  • Accept comma inside values only as delimiter between search and replace; only split on the first comma so replace strings may include commas. Trim whitespace.
  • Validate that a search value is non-empty; allow replace to be empty.
  • Use spawn with shell: false and pass arguments as arrays to avoid shell injection.
  • Add tracker events 'search_replace_start' and 'search_replace_done' around the operation.
  • Keep the existing command cancellation semantics (if existing run() returned false when canceled, preserve that behavior).

Testing considerations:

  • Confirm CLI accepts --search-replace and normalizes values in both single and multiple forms.
  • Confirm that when searchReplace is provided the command runs WP-CLI search-replace with the expected flags and succeeds when WP-CLI returns 0.
  • Confirm that multisite adds --network flag.

List of concrete changes to create in the PR (high level):

  • Modify src/bin/vip-dev-env-sync-sql.js to add CLI option and pass option through.
  • Add/modify src/commands/dev-env-sync-sql.js to implement logic described above. If the file already exists, update it; otherwise create it.

Please create a branch, make the changes, and open a pull request with a concise title and description summarizing the changes and rationale. Include notes in the PR description suggesting that if a project helper exists to run WP inside Lando, execWpCliInLando should be replaced with that helper for consistency.

Files/content to update (high-level - the agent should produce actual edits):

  • src/bin/vip-dev-env-sync-sql.js: add option block for [ 'r', 'search-replace' ], normalize opt.searchReplace to array, pass into DevEnvSyncSQLCommand as extraOptions.
  • src/commands/dev-env-sync-sql.js: implement the search-replace logic and helper functions as described.

Make the changes with clean, secure code and include basic inline comments where needed.

Do not modify unrelated files.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2025

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@sjinks
Copy link
Member

sjinks commented Nov 6, 2025

@copilot, I don't think we need wp search-replace. Please check the resolveImportPath() function and the DevEnvImportSQLCommand.run() method.

Copilot AI changed the title [WIP] Add search-replace option to vip dev-env sync sql command Add --search-replace option to vip dev-env sync sql Nov 6, 2025
Copilot AI requested a review from sjinks November 6, 2025 18:22
Copilot finished work on behalf of sjinks November 6, 2025 18:22
Use the existing search-replace mechanism on the SQL file (via @automattic/vip-search-replace)
instead of running WP-CLI search-replace after import. This matches the approach used in
dev-env-import-sql and is more efficient.

- Add --search-replace CLI option with array normalization
- User-provided pairs are added to searchReplaceMap in generateSearchReplaceMap()
- Existing runSearchReplace() method handles all replacements on SQL file
- Fix early return bug in generateSearchReplaceMap() to ensure user pairs are always added

Co-authored-by: sjinks <[email protected]>
Copy link
Contributor Author

Copilot AI commented Nov 6, 2025

@copilot, I don't think we need wp search-replace. Please check the resolveImportPath() function and the DevEnvImportSQLCommand.run() method.

You're absolutely right! I've refactored the implementation to use the SQL file search-replace approach (via @automattic/vip-search-replace) instead of WP-CLI search-replace after import. This matches how dev-env-import-sql handles it via resolveImportPath().

The new implementation:

  • Adds user-provided pairs to the existing searchReplaceMap
  • Uses the existing runSearchReplace() method that operates on the SQL file
  • More efficient and consistent with the import-sql approach

Changes in commit a046a41

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 6, 2025

Copilot finished work on behalf of sjinks November 6, 2025 18:33
public _track: TrackFunction;
private _sqlDumpType?: SqlDumpType;
public sdsSiteUrls: WpSite[] = [];
private userSearchReplacePairs?: string[];
Copy link
Member

Choose a reason for hiding this comment

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

@copilot, we don't reassign this property anywhere; can we make it readonly, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! Made the property readonly in commit d207359.

The property is only assigned in the constructor and never reassigned,
so it can be marked as readonly for better immutability.

Co-authored-by: sjinks <[email protected]>
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 6, 2025

Copilot AI requested a review from sjinks November 6, 2025 19:43
Copilot finished work on behalf of sjinks November 6, 2025 19:43
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