search-replace-command icon indicating copy to clipboard operation
search-replace-command copied to clipboard
verified publisher icon wp-cli

Replacement callback & row-based filtering

Open drzraf opened this issue 6 years ago • 13 comments

  • Reroll of #104 ( Feature: Call user function with --callback flag #104 )
  • Preliminary work for #127 (Callback access to current primary key & column)
  • Include #125 (Introduce --where flag)

drzraf avatar Sep 18 '19 15:09 drzraf

@drzraf Is this still a work-in-progress ? If not, can you do the necessary style changes to make PHPCS happy? Running vendor/bin/phpcbf should already fix a large portion of them.

See here: https://travis-ci.org/wp-cli/search-replace-command/jobs/586606309#L591-L709

schlessera avatar Sep 27 '19 05:09 schlessera

Just did^

drzraf avatar Apr 17 '20 14:04 drzraf

Could you please have a look. It was a huge MR update and I'll hardly find the motivation to further rework/update it again if merge-conflicts arised. It's also rare to have an opportunity window to test changes.

drzraf avatar Jul 01 '20 13:07 drzraf

ping

drzraf avatar Oct 22 '20 19:10 drzraf

@drzraf The code looks good so far, and I'd love to merge this. However, for merging it, we'll need tests as well that cover all the main code paths. Are you able to add such tests?

schlessera avatar Oct 29 '20 11:10 schlessera

Note: the Travis issues should be resolved by now.

schlessera avatar Oct 29 '20 11:10 schlessera

Hi @schlessera , I completely understand your concern given the shape of the diff. Sadly I'm not working with this command these days and have literally no-time I could dedicate to further refine the PR with test; sorry.

drzraf avatar Oct 29 '20 13:10 drzraf

Thanks for the quick response! I'll see if I can manage to add the tests myself to get this included.

schlessera avatar Oct 29 '20 13:10 schlessera

Would you merge if I were to rebase it once again?

drzraf avatar May 19 '22 13:05 drzraf

Has this seen any update at all lately? I need to find a solution to search-replace by excluding revisions. 🤔

davidwebca avatar Jul 08 '22 14:07 davidwebca

@davidwebca Want to open a new pull request with this code and tests covering the change?

danielbachhuber avatar Jul 08 '22 15:07 danielbachhuber

I'll a see what I can do!

davidwebca avatar Jul 08 '22 15:07 davidwebca

@danielbachhuber : What about providing writing tests since upstream are the ones who know how to best write them?

I'd later see and possibly spend the necessary hours for another rebase.

drzraf avatar Oct 03 '22 21:10 drzraf

Proceeding with https://github.com/wp-cli/wp-cli/issues/5594 for this repository. I've captured this PR to https://gist.github.com/danielbachhuber/39778759a59cb231c94520e38ae3f6ba in case this PR is auto-closed or broken in some way.

danielbachhuber avatar Nov 18 '22 17:11 danielbachhuber

@danielbachhuber Completely had forgotten about this. Do you have any guides or examples of tests (following up that conversation higher here?)

davidwebca avatar Nov 18 '22 18:11 davidwebca

@davidwebca Here's our guide to writing tests: https://make.wordpress.org/cli/handbook/contributions/pull-requests/#running-and-writing-tests

There are plenty of examples within this and other repos. Feel free to stop by #cli in WordPress.org Slack with any questions you might have.

Also, specific to this pull request: I think a filter-based approach would work best for this need.

danielbachhuber avatar Nov 18 '22 19:11 danielbachhuber