Make swiper-query-replace a standalone command

[astoff]

Apparently it's not possible to resume a previous swiper-query-replace operartion, so I would like to propose the attached change.

The candidates in the swiper-query-replace prompt will be the previous replacement texts used for the given replacement subject. The history in that query will be the full history of replacement texts.

I frankly never understood what that old :def parameter in the ivy-read call is supposed to be telling me, and I removed it. If I'm missing something clever here, let me know :-).

Another idea, which I could work on if there is interest, would be to modify swiper-query-replace so that it also makes sense when called outside of swiper. This would be a variant of swiper where hitting RET asks for a replacement text instead of just exiting the search. This is closer to the usual query-replace and might feel more familiar to some people.

[astoff]

I realized that providing candidates in the replacement text query isn't a great idea, because it's too easy to mistakenly select one of them instead of the current input. (If (ivy-read ... :preselect -1) was a thing, this could perhaps be fixed, but I don't think it would be the best UI anyway). So I removed that. It's still possible to access the history of replacement texts with C-r, which is already an improvement over the previous version.

More importantly, though, I implemented the idea sketched in my previous comment, and swiper-query-replace is now a standalone command and can be called from top level. It works as follows:

• If called during a Swiper search, it behaves just as before. (With the caveat that C-g will quit altogether, instead of going back to the Swiper search).
• For added discoverability, the above functionality can also be accessed as an ivy action, namely M-o q.
• If called from outside the minibuffer, swiper-query-replace is similar to swiper-isearch, except that
  1. ivy-done takes you to the state where a replacement text is requested.
  2. query-replace-from-history-variable is used instead of swiper-history.
  3. A query-replace in region is arranged for if the region is active.
  4. A special regex builder is used, whose effect is to not change at all the inserted regex. This is because in a query-replace, one probably wants precision over convenience.
  5. Allows resuming the last query-replace with a prefix arg.

There's still something awry with the overlays (sometimes they stick around after the command quits). Unfortunately that part of the code looks quite arcane to me. If you like the general idea, we can see what to do about that part.

[basil-conto]

Thanks. I can't say whether or in what form these changes will be accepted, but I just wanted to inform you that large contributions require copyright to be assigned to the Free Software Foundation, as Ivy is copyrighted by the FSF. Do you have such a CA on file already? If not, would you be willing to submit one? If you're unfamiliar with this, the process is relatively straightforward, and there's more info at (info "(emacs) Copyright Assignment"). You can find the form itself at https://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright/request-assign.future.

[astoff]

Of course I'm willing to sign the CA papers (which I haven't done so far), as long as there is some indication I'll need them :-)

[abo-abo]

Hi. The PR looks good to me. Please proceed with the CA 👍

[astoff]

Ok, I completed the CA paperwork.

I've also pushed a commit to refine the start point of the replacement. With that in place, I think the logic of the command is good, but of course I'm happy to discuss all the details.

It remains to sort out some issues with the overlays. I think you can address those things more effectively and reliably, @abo-abo. How should we organize this?

[basil-conto]

This gives a "reference to free variable swiper-isearch-map" byte-compiler warning, which probably means you need to define that variable before this function.

[astoff]

I've replaced this with swiper-map. Then M-n will not quote the query, but at least no large reorganizing of the code is needed.

[basil-conto]

I don't know which is better, but FWIW moving a defvar verbatim doesn't qualify as "large reorganizing of the code".

[astoff]

True; but then a random piece of swiper-isearch will be outside of the corresponding ;;*-section of the file. I was assuming this is undesired.

[basil-conto]

Rules are made to be broken ;).

[basil-conto]

Or rather, there are always exceptions that prove the rule.