Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 46 commits into from Feb 1, 2019
Merged

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

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
48a8fed
wip
aschlaep Dec 4, 2018
a1a84d2
remove files
aschlaep Dec 4, 2018
891ae5d
partially implemented search
aschlaep Dec 11, 2018
142166c
forward and reverse search mostly works
aschlaep Dec 12, 2018
d9408ea
improved search, need to reexamine indexing
aschlaep Dec 15, 2018
01804c0
mostly working, needs cleanup
aschlaep Dec 20, 2018
8aea812
wip: mainly feature complete, needs testing, UI, code cleanup
aschlaep Dec 22, 2018
7d3989e
live update index
aschlaep Jan 7, 2019
4f746d0
new UI, styling
aschlaep Jan 12, 2019
d94d150
minor cleanup
aschlaep Jan 12, 2019
544ae5e
styling updates, small bugfixes
aschlaep Jan 16, 2019
bf0ccbd
remove lastquery, change command category
aschlaep Jan 16, 2019
181bd04
manual initial codemirror scan to avoid issue with lazy loading searc…
aschlaep Jan 16, 2019
4d978f1
code review updates
aschlaep Jan 16, 2019
667a4e9
cleanup, comments
aschlaep Jan 17, 2019
744cefd
guard against non-searchable widgets, update overlay toggle logic
aschlaep Jan 28, 2019
58a81f9
cleanup per review
aschlaep Jan 28, 2019
42f4d73
add styling, resources
aschlaep Jan 28, 2019
14a318e
changes per jasons comments
aschlaep Jan 29, 2019
525b1a8
Change search registry to use a Map.
jasongrout Jan 29, 2019
272af34
move canSearchOn to static
aschlaep Jan 29, 2019
ad1396f
update licenses
aschlaep Jan 29, 2019
d76f1a8
private underscores
aschlaep Jan 29, 2019
5f206c3
Simplify search provider registry find function.
jasongrout Jan 29, 2019
e4ed607
bind -> arrow functions
jasongrout Jan 29, 2019
b1d95bb
Change the functions to take only widgets instead of shell.
jasongrout Jan 29, 2019
7b3bef3
Formatting
jasongrout Jan 29, 2019
184074e
Change signal to have a ‘this’ sender, move offset to SearchInstance
jasongrout Jan 29, 2019
d75e5c5
Move initializing steps into constructor.
jasongrout Jan 29, 2019
d5ac6fb
Break SearchInstance into its own file
aschlaep Jan 29, 2019
fb7f998
Make SearchInstance implement IDisposable
aschlaep Jan 29, 2019
53aea0f
async await SearchInstance
aschlaep Jan 29, 2019
046c830
switch to async/await in NBSearchProvider, fix off by one issues
aschlaep Jan 30, 2019
3478507
async await, update license, and minor refactor for CMSearchProvier, …
aschlaep Jan 30, 2019
9a0b5dc
remove private namespace in NBSearchProvider
aschlaep Jan 30, 2019
38224e7
escape to close, address focus issue, add docs to searchproviders
aschlaep Jan 31, 2019
f052f5c
Merge branch 'master' into pr/aschlaep/5795
jasongrout Jan 31, 2019
9e9eab5
Fix the search extension to use the new frontend objects.
jasongrout Jan 31, 2019
6955cb0
switch to start/endQuery and endSearch, made search lifetime tweaks, …
aschlaep Feb 1, 2019
0d048a5
Merge branch 'ctrl-f-init' of github.com:aschlaep/jupyterlab into ctr…
aschlaep Feb 1, 2019
52c75fb
fix toggle styling, fix bug on deleted md cell containing match
aschlaep Feb 1, 2019
de093b1
Take care of a few more lifecycle and enabling issues; add commands t…
jasongrout Feb 1, 2019
337a628
A bit more cleanup.
jasongrout Feb 1, 2019
8bef9a2
Fix dependencies.
jasongrout Feb 1, 2019
ea774b3
Fix codemirror typings for editor test.
jasongrout Feb 1, 2019
31e0484
Fix lint error.
jasongrout Feb 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions dev_mode/package.json
View file
Open in desktop
Expand Up @@ -31,6 +31,7 @@
"@jupyterlab/docmanager": "^0.19.1",
"@jupyterlab/docmanager-extension": "^0.19.1",
"@jupyterlab/docregistry": "^0.19.1",
"@jupyterlab/documentsearch-extension": "^0.19.1",
"@jupyterlab/extensionmanager": "^0.19.1",
"@jupyterlab/extensionmanager-extension": "^0.19.1",
"@jupyterlab/faq-extension": "^0.19.1",
Expand Down Expand Up @@ -143,6 +144,7 @@
"@jupyterlab/console-extension": "",
"@jupyterlab/csvviewer-extension": "",
"@jupyterlab/docmanager-extension": "",
"@jupyterlab/documentsearch-extension": "",
"@jupyterlab/extensionmanager-extension": "",
"@jupyterlab/faq-extension": "",
"@jupyterlab/filebrowser-extension": "",
Expand Down Expand Up @@ -246,6 +248,7 @@
"@jupyterlab/docmanager": "../packages/docmanager",
"@jupyterlab/docmanager-extension": "../packages/docmanager-extension",
"@jupyterlab/docregistry": "../packages/docregistry",
"@jupyterlab/documentsearch-extension": "../packages/documentsearch-extension",
"@jupyterlab/extensionmanager": "../packages/extensionmanager",
"@jupyterlab/extensionmanager-extension": "../packages/extensionmanager-extension",
"@jupyterlab/faq-extension": "../packages/faq-extension",
Expand Down
2 changes: 1 addition & 1 deletion examples/console/package.json
View file
Open in desktop
Expand Up @@ -18,7 +18,7 @@
"es6-promise": "~4.1.1"
},
"devDependencies": {
"@types/codemirror": "~0.0.46",
"@types/codemirror": "~0.0.70",
"css-loader": "~0.28.7",
"file-loader": "~1.1.11",
"mini-css-extract-plugin": "~0.4.4",
Expand Down
2 changes: 1 addition & 1 deletion examples/filebrowser/package.json
View file
Open in desktop
Expand Up @@ -22,7 +22,7 @@
"es6-promise": "~4.1.1"
},
"devDependencies": {
"@types/codemirror": "~0.0.46",
"@types/codemirror": "~0.0.70",
"css-loader": "~0.28.7",
"file-loader": "~1.1.11",
"mini-css-extract-plugin": "~0.4.4",
Expand Down
2 changes: 1 addition & 1 deletion examples/notebook/package.json
View file
Open in desktop
Expand Up @@ -21,7 +21,7 @@
"es6-promise": "~4.1.1"
},
"devDependencies": {
"@types/codemirror": "~0.0.46",
"@types/codemirror": "~0.0.70",
"css-loader": "~0.28.7",
"file-loader": "~1.1.11",
"mini-css-extract-plugin": "~0.4.4",
Expand Down
2 changes: 1 addition & 1 deletion packages/codemirror/package.json
View file
Open in desktop
Expand Up @@ -46,7 +46,7 @@
"react": "~16.4.2"
},
"devDependencies": {
"@types/codemirror": "~0.0.46",
"@types/codemirror": "~0.0.70",
"rimraf": "~2.6.2",
"typedoc": "~0.12.0",
"typescript": "~3.1.1"
Expand Down
69 changes: 66 additions & 3 deletions packages/codemirror/src/editor.ts
View file
Open in desktop
@@ -1,5 +1,7 @@
// Copyright (c) Jupyter Development Team.
// Distributed under the terms of the Modified BSD License.
/// <reference types="codemirror"/>
/// <reference types="codemirror/searchcursor"/>

import CodeMirror from 'codemirror';

Expand Down Expand Up @@ -369,6 +371,65 @@ export class CodeMirrorEditor implements CodeEditor.IEditor {
this._clearHover();
}

// todo: docs, maybe define overlay options as a type?
addOverlay(mode: string | object, options?: object): void {
this._editor.addOverlay(mode, options);
}

removeOverlay(mode: string | object): void {
this._editor.removeOverlay(mode);
}

getSearchCursor(
query: string | RegExp,
start?: CodeMirror.Position,
caseFold?: boolean
): CodeMirror.SearchCursor {
return this._editor.getDoc().getSearchCursor(query, start, caseFold);
}

getCursor(start?: string): CodeMirror.Position {
return this._editor.getDoc().getCursor(start);
}

get state(): any {
return this._editor.state;
}

operation<T>(fn: () => T): T {
return this._editor.operation(fn);
}

firstLine(): number {
return this._editor.getDoc().firstLine();
}

lastLine(): number {
return this._editor.getDoc().lastLine();
}

scrollIntoView(
pos: { from: CodeMirror.Position; to: CodeMirror.Position },
margin: number
): void {
this._editor.scrollIntoView(pos, margin);
}

cursorCoords(
where: boolean,
mode?: 'window' | 'page' | 'local'
): { left: number; top: number; bottom: number } {
return this._editor.cursorCoords(where, mode);
}

getRange(
from: CodeMirror.Position,
to: CodeMirror.Position,
seperator?: string
): string {
return this._editor.getDoc().getRange(from, to, seperator);
}

/**
* Add a keydown handler to the editor.
*
Expand Down Expand Up @@ -408,7 +469,7 @@ export class CodeMirrorEditor implements CodeEditor.IEditor {
*/
revealSelection(selection: CodeEditor.IRange): void {
const range = this._toCodeMirrorRange(selection);
this._editor.scrollIntoView(range);
this._editor.scrollIntoView(range); // TODO
}

/**
Expand Down Expand Up @@ -760,8 +821,10 @@ export class CodeMirrorEditor implements CodeEditor.IEditor {
*/
private _toCodeMirrorRange(range: CodeEditor.IRange): CodeMirror.Range {
return {
from: this._toCodeMirrorPosition(range.start),
to: this._toCodeMirrorPosition(range.end)
anchor: this._toCodeMirrorPosition(range.start),
head: this._toCodeMirrorPosition(range.end),
from: this._toCodeMirrorPosition.bind(this, range.start),
to: this._toCodeMirrorPosition.bind(this, range.end)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
     };
   }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

};
}

Expand Down
1 change: 1 addition & 0 deletions packages/codemirror/src/typings.d.ts
View file
Open in desktop
Expand Up @@ -2,3 +2,4 @@
// Distributed under the terms of the Modified BSD License.

/// <reference path="../typings/codemirror/codemirror.d.ts"/>
/// <reference types="@types/codemirror/searchcursor"/>