The following comes from my comment on #3 but never got turned into an issue, so here it is in an issue.

First of all, it's been a while since this PR came out and we've since made at least 1 change to the advancement logic, so the conflicts are real and it definitely can't be merged as-is. (Sorry for the delay! It took a while to flush out other issues and then ruminate on them.)

Second, I want to fix all our issues with advancement at once rather than doing one case at a time. We need an advancement policy for each of the actions the user can take:

accept replacement (y)
reject replacement and continue (n)
open editor (e)
accept all replacements (A)
apply replacement and open editor (E)

I'll just talk through each of these cases below, keeping fastmod's philosophy in mind:

fastmod's major philosophical difference from codemod is that it is focused on improving the use case "I want to use interactive mode to make sure my regex is correct, and then I want to apply the regex everywhere"

Accept replacement (y)

Here, our current policy is to advance 0 characters if the replacement has length 0 and 1 character otherwise. We've already rejected the policy "advance 1 character always" (see 501a8b9484177b869124df93238bf45a85ccad78 and 95ae97aece677d1d72ed501e2f33a9cb73bf7dcd). Advancing 1 character has the advantage that it allows you to do further editing on replacement text, but given fastmod's philosophy of driving toward automated application of your regex everywhere, this advantage has minimal value.

Proposed new policy: advance to the next character after the end of the replacement text.

Reject replacement (n)

We have two policies to choose from when a replacement is rejected: 1) We can advance 1 character and try again (existing behavior), or 2) we can advance to the end of the regex match and try again (@steven807's proposal in this PR). I think the answer to the tradeoff has to come from fastmod's philosophy.

If you say n at fastmod's interactive prompt without quitting, you're already outside our core use case. We're left trying to guess if your intent is "hmm, my regex looks wrong, but I want to keep searching to make sure I continue to see nonsense" (which would suggest policy 2) or "I do want to do this replacement, but the start position is wrong, so let's manually move over 1 character and try again" (which would suggest policy 1). Given our philosophy, I think @steven807 is right that we should move to policy 2 in the rejection case.

Proposed new policy: advance to the next character after the end of the match.

Open editor (e)

Our current policy is to advance 1 character and keep going.

The fundamental problem with the open editor option is that the user could arbitrarily change the document if they wanted to, and since this is outside our core use case, we're never going to have a great picture of their change. I propose that we should assume that the user is "working with us" by only editing within the matched region, may or may not have actually applied any changes, and wants to continue to the next match when they start interacting with fastmod again. Given these assumptions, I think our existing policy makes sense.

Proposed new policy: no change.

Apply replacement and open editor (E)

Our current policy is to advance 1 character and keep going. Since interactive editing is not our core use case, I think we should not worry about the difference between E and e.

Proposed new policy: no change.

Accept all replacements (A)

Our current policy is to act as though the user pressed y repeatedly. There is an infinite loop bug with A on master for both codemod and fastmod:

$ echo 'foo foo' > /tmp/test.txt
$ cd /tmp
$ codemod -m foo barfoo --extension txt
press A to accept all interactively

The problem there is that the replacement matches the search pattern, so we continually replace our own replacement (and grow it).

However, given the proposed policy changes for y, I think we can keep the policy for A as-is.

Proposed new policy: no direct change (act as though y was pressed at every subsequent prompt), but inherit the changes in the y policy.

Next steps

The next step is to implement the new policy. @steven807, if you want to do that, that's great, but I totally understand if the moment has passed.

Originally posted by @swolchok in https://github.com/facebookincubator/fastmod/pull/3#issuecomment-477737053

facebookincubator / fastmod

Implement unified advancement policy #17