Add ctrl-f/g support for notebooks and text files

[aschlaep]

Fixes #1074

Goals

This is the start of a document search framework. The goal is to provide default search functionality for notebooks and text documents, but provide the option to override the default behavior or add custom functionality for custom widgets.

What this does

This PR creates the document search extension as a starting point for these search goals. It provides a default implementation for searching notebook inputs (code and markdown) as well as text documents (opened in codemirror). There are two search providers in this PR, one for codemirror and one for notebooks (which uses the codemirror search provider). The default functionality provided includes searching forward and backward, counting matches, tracking match index, case sensitive search, and regex search. I was able to make use of codemirror's searchcursor and overlay functionality, but had to mostly rewrite its search.js to provide my own hooks and match tracking.

Current behavior is that when starting a search, it'll look through all of your code and markdown cells for matches. It'll unhide any hidden inputs containing matches and unrender any markdown containing matches. It'll highlight all matches and put the cursor on the next match after the last cursor position in the active cell. Once done searching, it'll re-render the markdown cells that were temporarily un-rendered (I chose not to re-hide code cells, but that could easily change). The search can then toggle through matches across cells and loop at the beginning/end of the notebook. If you move your cursor mid-search, it'll start toggling through matches from the new cursor position. Thanks to codemirror's awesome overlay, the highlighting will update live as you type.

Also in this PR, I removed the custom types living in the codemirror package in favor of those that exist in @types/codemirror. I added back the types that were used by JupyterLab and do not exist in DefinitelyTyped's set of types. I also had to expose several additional functions on codemirror to perform the search, get the cursor, etc.

Future work

Potential (hopefully near-term) future enhancements to this would include search & replace, searching outputs, and searching across multiple files.

To do:

• replace UI used for testing with a proper overlay above active document
• discuss interfaces
• reorganize the commands
• address case of adding/deleting cells mid-search
• ensure search works with multiple documents open
• more testing/bug squashing

Here's a gif demo with the UI I've been using for testing.

[jasongrout]

@aschlaep - here is the recent work on React widgets: #5479

That PR is a bit overwhelming, but when you get a chance to push some of your react code up here, we can take a look at how to use this new react infrastructure.

[ivanov]

some initial comments, have to wrap my head around the larger design of this...

[ivanov]

is there a reason that anchor and head can use the function invocation way, but from and to need the bind? Seems like these four should be the same, or could use a comment explaining why they are different.

[aschlaep]

For some reason, CodeMirror's Range type takes anchor and head as values and from and to as functions. I'll have to double check my assumptions here about the values desired here. There's a bit of annoying translation needed in some places between the CodeMirror Range type and the CodeEditor IRange.

[jasongrout]

from and to are functions that basically pick off the min and max of the anchor/head. See https://github.com/codemirror/CodeMirror/blob/5b949b2dbe92a0153c2bd32b75341e7fe3cf6ce0/src/model/selection.js#L51-L59

[jasongrout]

Perhaps we should just be using the CodeMirror Range object directly here? Can we import and use it?

[jasongrout]

Actually, I was reading it wrong. I think you are eventually going to call this function: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/89592f7ec313174363136fe91749cedff272a70e/types/codemirror/index.d.ts#L294

That function needs a {from: CodeMirror.Position, to: CodeMirror.Position} range, not a CodeMirror.Range object. So I think we perhaps don't need this private function after all?

[jasongrout]

Or at least, if we do want this private function, it is not returning a CodeMirror.Range, just a {from: position, to: position} object.

I can understand the confusion. The CodeMirror docs say it takes a range, but looking at the codemirror source, it's clear it wasn't talking about a CodeMirror.Range object, but rather a simple object with from and to positions (not functions).

[jasongrout]

I think a patch like this should make this bit of code work:

diff --git a/packages/codemirror/src/editor.ts b/packages/codemirror/src/editor.ts
index 9f3cf38c7..fb10dec89 100644
--- a/packages/codemirror/src/editor.ts
+++ b/packages/codemirror/src/editor.ts
@@ -469,7 +469,8 @@ export class CodeMirrorEditor implements CodeEditor.IEditor {
    */
   revealSelection(selection: CodeEditor.IRange): void {
     const range = this._toCodeMirrorRange(selection);
-    this._editor.scrollIntoView(range);
+    // Explicitly pass in undefined to select the right function typing overload.
+    this._editor.scrollIntoView(range, undefined);
   }
 
   /**
@@ -819,12 +820,12 @@ export class CodeMirrorEditor implements CodeEditor.IEditor {
   /**
    * Converts an editor selection to a code mirror selection.
    */
-  private _toCodeMirrorRange(range: CodeEditor.IRange): CodeMirror.Range {
+  private _toCodeMirrorRange(
+    range: CodeEditor.IRange
+  ): { from: CodeMirror.Position; to: CodeMirror.Position } {
     return {
-      anchor: this._toCodeMirrorPosition(range.start),
-      head: this._toCodeMirrorPosition(range.end),
-      from: this._toCodeMirrorPosition.bind(this, range.start),
-      to: this._toCodeMirrorPosition.bind(this, range.end)
+      from: this._toCodeMirrorPosition(range.start),
+      to: this._toCodeMirrorPosition(range.end)
     };
   }

[jasongrout]

@aschlaep - I think this should work for now, and we can delete the extra argument to the scrollIntoView if we submit a change to the codemirror typings to make the margin argument optional in https://github.com/DefinitelyTyped/DefinitelyTyped/blob/ee0aad13bc3dd0f0176ca6794aacc0227d88c0fe/types/codemirror/index.d.ts#L286 and https://github.com/DefinitelyTyped/DefinitelyTyped/blob/ee0aad13bc3dd0f0176ca6794aacc0227d88c0fe/types/codemirror/index.d.ts#L294

[aschlaep]

Put together a UI that's closer to the final thing (need some proper up/down arrows, some minor alignment tweaks), here's a quick demo.

[aschlaep]

This is now ready for review. I'm still missing the resources for the buttons, but that should be a minor update (hopefully tomorrow). Near-term future work for the document search extension include writing tests, focus tweaks (focus the document after closing search), perhaps switching the search box to a toolbar widget, and beginning work on search/replace and whole workspace search. @jasongrout

[jasongrout]

Andrew, I just merged in master and fixed the search extension to work with master. It was indeed a simple renaming.

[jasongrout]

@aschlaep - ready for final review?

[aschlaep]

Ready now! Caught one more bug and made a styling update, it looks like something I merged in caused the button toggles to get box-sizing: unset, which caused them to get bigger and repeat their background, so I set the buttons to no background repeat as well as setting the input toggles to box-sizing: border-box, which seemed to fix the issue in both chrome and firefox. The bug was an error that occurred when it tried to re-render a markdown cell containing a match after that cell had been deleted.

[gnestor]

I'd be happy to user test when it's ready 👍

[jasongrout]

Grant, if you want to pull this pr and test, that would be awesome

[gnestor]

I tried checking out the branch and jlpming and running lab in dev-mode watch mode, but I don't see the find left sidebar panel...

[jasongrout]

Oh, it's better now! Ctrl f or use the command palette

[gnestor]

Oh ya! First impression: the UX is nice!

[jasongrout]

Okay, I think this is ready to go in. Two follow-up issues I noticed for the next iteration:

(a) I noticed when I had flux running, I could no longer see the yellow highlight. This was really confusing. It would be great to see if the highlight disappears in other night-mode blue-light-disabled schemes. Actually, it'd be nice if the highlight color was customizable, or at least if it used the standard jlab theme colors so that it worked well with different themes.
(b) I noticed in vs code and sublime, the search query was kept around even when the search box was not shown, so that find next/previous still worked with the last query. I played a bit with having the search instance stick around, and just hide/show the search box as needed, but didn't get very far before deciding that was for the next iteration.

[jasongrout]

@aschlaep - thanks very much again! Merging this, let's iterate from here.

[cquah]

Thank you @aschlaep @jasongrout !!

[aschlaep]

Awesome, thanks @jasongrout!