cleanup, comments

dev_mode/package.json

@@ -31,7 +31,7 @@
     "@jupyterlab/docmanager": "^0.19.1",
     "@jupyterlab/docmanager-extension": "^0.19.1",
     "@jupyterlab/docregistry": "^0.19.1",
-    "@jupyterlab/documentsearch-extension": "^0.7.1",
+    "@jupyterlab/documentsearch-extension": "^0.19.1",
     "@jupyterlab/extensionmanager": "^0.19.1",
     "@jupyterlab/extensionmanager-extension": "^0.19.1",
     "@jupyterlab/faq-extension": "^0.19.1",

packages/codemirror/typings/codemirror/codemirror.d.ts

@@ -5,6 +5,10 @@
 
 import * as CodeMirror from 'codemirror';
 
+/**
+ * Define extra codemirror types that do not exist in the DefinitelyTyped
+ * type resources
+ */
 declare module 'codemirror' {
   /**
    * id will be the id for the defined mode. Typically, you should use this second argument to defineMode as your module scope function

packages/documentsearch-extension/schema/plugin.json

@@ -5,17 +5,17 @@
     {
       "command": "documentsearch:start",
       "keys": ["Accel F"],
-      "selector": ".jp-Document"
+      "selector": "body"
     },
     {
       "command": "documentsearch:highlightNext",
       "keys": ["Accel G"],
-      "selector": ".jp-Document"
+      "selector": "body"
     },
     {
       "command": "documentsearch:highlightPrevious",
       "keys": ["Accel Shift G"],
-      "selector": ".jp-Document"
+      "selector": "body"
     }
   ],
   "properties": {},

packages/documentsearch-extension/src/index.ts

@@ -98,14 +98,45 @@ export interface ISearchProvider {
   readonly currentMatchIndex: number;
 }
 
-export interface IDisplayUpdate {
+export interface IDisplayState {
+  /**
+   * The index of the currently selected match
+   */
   currentIndex: number;
+
+  /**
+   * The total number of matches found in the document
+   */
   totalMatches: number;
+
+  /**
+   * Should the search be case sensitive?
+   */
   caseSensitive: boolean;
+
+  /**
+   * Should the search string be treated as a RegExp?
+   */
   useRegex: boolean;
+
+  /**
+   * The text in the entry
+   */
   inputText: string;
+
+  /**
+   * The query constructed from the text and the case/regex flags
+   */
   query: RegExp;
+
+  /**
+   * An error message (used for bad regex syntax)
+   */
   errorMessage: string;
+
+  /**
+   * Should the focus forced into the input on the next render?
+   */
   forceFocus: boolean;
 }
 
@@ -167,11 +198,6 @@ class SearchInstance {
     const toolbarHeight = (this._widget as DocumentWidget).toolbar.node
       .clientHeight;
     this.initializeSearchAssets(registry, toolbarHeight);
-    // check the full content of the cm editor on start search, see if full content
-    // is there or lazy loaded...  is it a codemirror setting?  can i force full load?
-    // I don't think it's a lazy load issue, i think the changed event may not be getting fired?
-    // need to evaluate if the search overlay is the one not being evaluated or if the changed event
-    // isn't getting fired after the final update
   }
 
   get searchWidget() {
@@ -194,8 +220,8 @@ class SearchInstance {
   }
 
   private _widget: Widget;
-  private _displayState: IDisplayUpdate;
-  private _displayUpdateSignal: Signal<ISearchProvider, IDisplayUpdate>;
+  private _displayState: IDisplayState;
+  private _displayUpdateSignal: Signal<ISearchProvider, IDisplayState>;
   private _activeProvider: ISearchProvider;
   private _searchWidget: Widget;
   private updateDisplay() {
@@ -223,7 +249,6 @@ class SearchInstance {
   }
   private endSearch() {
     this._activeProvider.endSearch().then(() => {
-      // more cleanup probably
       Signal.disconnectAll(this);
       this._searchWidget.dispose();
       this._activeProvider.changed.disconnect(this.updateIndices, this);
@@ -242,7 +267,7 @@ class SearchInstance {
     toolbarHeight: number
   ) {
     this._activeProvider = registry.getProviderForWidget(this._widget);
-    this._displayUpdateSignal = new Signal<ISearchProvider, IDisplayUpdate>(
+    this._displayUpdateSignal = new Signal<ISearchProvider, IDisplayState>(
       this._activeProvider
     );
 

packages/documentsearch-extension/src/providers/codemirrorsearchprovider.ts

@@ -112,7 +112,6 @@ export class CodeMirrorSearchProvider implements ISearchProvider {
   }
 
   private _refreshOverlay(query: RegExp) {
-    // multiple lines added
     const state = Private.getSearchState(this._cm);
     this._cm.operation(() => {
       state.query = query;
@@ -261,10 +260,13 @@ namespace Private {
         !caseSensitive
       );
       if (!cursor.find(reverse)) {
+        // if we don't want to loop, no more matches found, reset the cursor and exit
         if (!shouldLoop) {
           cm.setCursorPosition(position);
           return null;
         }
+
+        // if we do want to loop, try searching from the bottom/top
         const startOrEnd = reverse
           ? CodeMirror.Pos(cm.lastLine())
           : CodeMirror.Pos(cm.firstLine(), 0);

packages/documentsearch-extension/src/providers/notebooksearchprovider.ts

@@ -22,6 +22,8 @@ export class NotebookSearchProvider implements ISearchProvider {
     this._searchTarget = searchTarget;
     const cells = this._searchTarget.content.widgets;
 
+    // Listen for cell model change to redo the search in case of
+    // new/pasted/deleted cells
     const cellModel = this._searchTarget.model.cells;
     Signal.disconnectBetween(cellModel, this);
     cellModel.changed.connect(
@@ -38,10 +40,14 @@ export class NotebookSearchProvider implements ISearchProvider {
       const cmEditor = cell.editor as CodeMirrorEditor;
       const cmSearchProvider = new CodeMirrorSearchProvider();
       cmSearchProvider.shouldLoop = false;
-      let reRenderPlease = false;
+
+      // If a rendered MarkdownCell contains a match, unrender it so that
+      // CodeMirror can show the match(es).  Keep track so that the cell
+      // can be rerendered when the search is ended.
+      let cellShouldReRender = false;
       if (cell instanceof MarkdownCell && cell.rendered) {
         cell.rendered = false;
-        reRenderPlease = true;
+        cellShouldReRender = true;
       }
       if (cell.inputHidden) {
         cell.inputHidden = false;
@@ -51,15 +57,16 @@ export class NotebookSearchProvider implements ISearchProvider {
         cmSearchProvider
           .startSearch(query, cmEditor)
           .then((matchesFromCell: ISearchMatch[]) => {
-            // update the match indices to reflect the whole document index values
             if (cell instanceof MarkdownCell) {
               if (matchesFromCell.length !== 0) {
                 // un-render markdown cells with matches
                 this._unRenderedMarkdownCells.push(cell);
-              } else if (reRenderPlease) {
+              } else if (cellShouldReRender) {
                 cell.rendered = true;
               }
             }
+
+            // update the match indices to reflect the whole document index values
             matchesFromCell.forEach(match => {
               match.index = match.index + indexTotal;
             });
@@ -210,6 +217,7 @@ namespace Private {
       ? provider.highlightPrevious()
       : provider.highlightNext();
     return nextPromise.then((match: ISearchMatch) => {
+      // If there was no match in this cell, try the next cell
       if (!match) {
         let nextCellIndex = getNextCellIndex(
           notebook.widgets,
@@ -218,7 +226,8 @@ namespace Private {
         );
         notebook.activeCellIndex = nextCellIndex;
         const editor = notebook.activeCell.editor as CodeMirrorEditor;
-        // move the cursor of the next cell to the start/end of the cell so it can search
+        // move the cursor of the next cell to the start/end of the cell so it can
+        // search the whole thing
         const newPosCM = reverse
           ? CodeMirror.Pos(editor.lastLine())
           : CodeMirror.Pos(editor.firstLine(), 0);

packages/documentsearch-extension/src/searchoverlay.tsx

@@ -4,7 +4,7 @@ import '../style/index.css';
 import { ReactWidget, UseSignal } from '@jupyterlab/apputils';
 import { Signal } from '@phosphor/signaling';
 import { Widget } from '@phosphor/widgets';
-import { IDisplayUpdate, ISearchProvider } from '.';
+import { IDisplayState, ISearchProvider } from '.';
 
 const OVERLAY_CLASS = 'jp-DocumentSearch-overlay';
 const INPUT_CLASS = 'jp-DocumentSearch-input';
@@ -124,7 +124,7 @@ function SearchIndices(props: ISearchIndexProps) {
 }
 
 interface ISearchOverlayProps {
-  overlayState: IDisplayUpdate;
+  overlayState: IDisplayState;
   onCaseSensitiveToggled: Function;
   onRegexToggled: Function;
   onHightlightNext: Function;
@@ -135,7 +135,7 @@ interface ISearchOverlayProps {
 
 class SearchOverlay extends React.Component<
   ISearchOverlayProps,
-  IDisplayUpdate
+  IDisplayState
 > {
   constructor(props: ISearchOverlayProps) {
     super(props);
@@ -217,8 +217,8 @@ class SearchOverlay extends React.Component<
 }
 
 export function createSearchOverlay(
-  wigdetChanged: Signal<ISearchProvider, IDisplayUpdate>,
-  overlayState: IDisplayUpdate,
+  wigdetChanged: Signal<ISearchProvider, IDisplayState>,
+  overlayState: IDisplayState,
   onCaseSensitiveToggled: Function,
   onRegexToggled: Function,
   onHightlightNext: Function,

packages/metapackage/package.json

@@ -49,7 +49,7 @@
     "@jupyterlab/docmanager": "^0.19.1",
     "@jupyterlab/docmanager-extension": "^0.19.1",
     "@jupyterlab/docregistry": "^0.19.1",
-    "@jupyterlab/documentsearch-extension": "^0.7.1",
+    "@jupyterlab/documentsearch-extension": "^0.19.1",
     "@jupyterlab/extensionmanager": "^0.19.1",
     "@jupyterlab/extensionmanager-extension": "^0.19.1",
     "@jupyterlab/faq-extension": "^0.19.1",