Expose API for extensions to extend find/replace

[SonOfLilit]

@rebornix judging by similar tickets this is in your domain.

I designed a nicer regex syntax and wrote a vscode extension to allow people to use it to search and replace (but while googling for how to do it, I encountered people asking about many similar use cases):

https://marketplace.visualstudio.com/items?itemName=sonoflilit.kleenexp https://github.com/SonOfLilit/kleenexp/blob/master/vscode/kleenexp/src/extension.ts

Unfortunately the UX is forced to be rather cumbersome. In particular, I can't show a nice Find widget (nice to have) and I can't implement search-as-you-type (showstopper).

Therefore, I'd like to volunteer to design and implement built in commands that will enable all kinds of Find/Replace extension use cases. Tentative design (to be formalized and reviewed):

Phase I

editor.actions.findWithArgs (link) as well as editor.action.previousMatchFindAction and editor.action.previousMatchFindAction would expose a revealWidget boolean parameter (so extensions could e.g. change the search string without revealing the search widget, and have their own buttons for F3/Shift+F3 that still don't show the UI)

Phase II

Design some way to hook more deeply into the search and replace behavior, so that extensions could override the behavior of all search/replace widgets in vscode. Since preprocessing the search string is the only use case I can imagine, maybe we should limit it to registering a function that preprocesses the search string. From my reading of the documentation, there is no mechanism to let extensions register functions to be called by vscode at a given event and return a value, so I'm not sure you'll even want to approve this, but it would be really great for my extension at least. I guess I'll just come back to you if and when a significant percent of users want my syntax everywhere.

Phase III

Implement syntax highlighting for the search widget (already in the backlog as #147726) in a way that would support both legacy syntax and setting a different language. Maybe even detecting groups, coloring groups in matches, and thus turning vscode into regex101.com and making users very very happy:

Yes, I would volunteer to implement this, but it will be very nontrivial (e.g. because Javascript regular expressions don't give you start/end indexes for matched groups) and will require some Product involvement (e.g. which colors to use for highlighting different groups?).

Summary

Anyway, that is the dream, let's turn back to reality. If I submit a design proposal for Phase I, will it get reviewed, and will a patch I submit based on an agreed design be accepted?

[rebornix]

@SonOfLilit thank you for all the suggestions and great write up, I really like how you separate the requests into multiple phases.

Phase 1 is straightforward, and it makes sense to me to expose arguments to allow extension to reveal the widget, we only need to double-check if we have other commands archiving the same behavior, if not, PRs are always welcome.

For Phase 2 and 3, I would probably swap them since Design some way to hook more deeply into the search and replace behavior would involve both team-wise API and UX discussions, especially when we are unifying the Find/Replace experience across the whole VS Code workbench. It would take long to move the needle.

Implement syntax highlighting for the search widget

This is very valuable IMHO, and we can archive this by using Monaco editor in the find input box, but I'm not sure if it's over kill to render a Monaco editor inside another Monaco editor. With that said, I'm open to explorations on this and see how far we can go with a solid regex language syntax highlighting and validator.

[SonOfLilit]

Is my current design for Phase I acceptable as is? Should I start writing code?

[rebornix]

@SonOfLilit it would be my honor to review your PR.

[SonOfLilit]

I'm having some complex trouble implementing what I want:

I would like to have my extension replicate this._editor.getOption(EditorOption.find).cursorMoveOnType behavior.

How this works for the find widget: input to the search field triggers FindReplaceState._onFindReplaceStateChange, which reaches this code, which chooses one of four ways to call this.research(e.moveCursor, ....):

    private _onStateChanged(e: FindReplaceStateChangedEvent): void {
[..]
        if (e.searchString || e.isReplaceRevealed || e.isRegex || e.wholeWord || e.matchCase || e.searchScope) {
            const model = this._editor.getModel();

            if (model.isTooLargeForSyncing()) {
[..]
            } else {
                if (e.searchScope) {
                    this.research(e.moveCursor, this._state.searchScope);
                } else {
                    this.research(e.moveCursor);
                }
            }
        }

However, when we change things with an editor command, CommonFindController.start() gets called, which does this:

    protected async _start(opts: IFindStartOptions, newState?: INewFindReplaceState): Promise<void> {
        this.disposeModel();
[..]
        this._state.change(stateChanges, false);  // false is for .moveCursor

        if (!this._model) { // always true afaict because of the this.disposeModel()
            this._model = new FindModelBoundToEditorModel(this._editor, this._state);
        }
        }

Even if I change false there to opts.moveCursor, it won't do what I want because new FindModelBoundToEditorModel() overrides it by calling this.research(false, ...).

Do you have a tip for me, or should I just keep playing with it until it works?

[SonOfLilit]

OK, I just added a moveCursor optional parameter to FindModelBoundToEditorModel. It works good enough that I don't care to get it perfect.

[SonOfLilit]

While working on this I stumbled on this change, which I think fixes a bug, but since I ended up choosing a different path it's not needed for my code and not part of the PR.

diff --git a/src/vs/editor/contrib/find/browser/findController.ts b/src/vs/editor/contrib/find/browser/findController.ts
index a91fce2fac5..5251c47bb12 100644
--- a/src/vs/editor/contrib/find/browser/findController.ts
+++ b/src/vs/editor/contrib/find/browser/findController.ts
@@ -603,7 +603,7 @@ export class StartFindWithArgsAction extends EditorAction {

                        await controller.start({
                                forceRevealReplace: false,
-                               seedSearchStringFromSelection: (controller.getState().searchString.length === 0) && editor.getOption(EditorOption.find).seedSearchStringFromSelection !== 'never' ? 'single' : 'none',
+                               seedSearchStringFromSelection: (args?.searchString?.length === 0) && editor.getOption(EditorOption.find).seedSearchStringFromSelection !== 'never' ? 'single' : 'none',
                                seedSearchStringFromNonEmptySelection: editor.getOption(EditorOption.find).seedSearchStringFromSelection === 'selection',
                                seedSearchStringFromGlobalClipboard: true,
                                shouldFocus: args?.shouldReveal === false ? FindStartFocusAction.NoReveal : FindStartFocusAction.FocusFindInput,

[SonOfLilit]

That bugfix turns out to have been indeed important, I was running tests the wrong way. Pushed it.

[SonOfLilit]

Hi,

My users are complaining about this bug in vscode that I caught and fixed as part of this PR, should I separate it into a smaller PR so it gets CR'd and merged faster?

https://github.com/microsoft/vscode/pull/156796/files#diff-c2c0a3f71ed0c733b8da6fe63824ea422fdbc76b47b82ab4afb772c4b2d11082R604

(it's checking controller.getState().searchString to see if it should seed search string from cursor, and ignoring the new searchString that we might have just passed. The effect is that the first time you search a document with my extension, it ignores what you typed and searches what's under the cursor, but from then on it behaves correctly)

[amunger]

@SonOfLilit - sorry for letting this hang for so long. If you still want to get that PR merged, can you try and update it again to resolve the conflicts and I will help get it reviewed and merged.