styling updates, small bugfixes

packages/documentsearch-extension/src/executor.ts

@@ -1,87 +0,0 @@
-import { SearchProviderRegistry } from './searchproviderregistry';
-import { ISearchMatch, ISearchProvider } from './index';
-
-import { ISignal, Signal } from '@phosphor/signaling';
-import { Widget } from '@phosphor/widgets';
-
-export class Executor {
-  constructor(registry: SearchProviderRegistry, activeWidget: Widget) {
-    this._registry = registry;
-    this._currentWidget = activeWidget;
-  }
-  startSearch(options: RegExp): Promise<ISearchMatch[]> {
-    // TODO: figure out where to check if the options have changed
-    // to know how to respond to an 'enter' keypress (new search or next search)
-    let cleanupPromise = Promise.resolve();
-    if (this._activeProvider) {
-      cleanupPromise = this._activeProvider.endSearch();
-    }
-
-    const provider = this._registry.getProviderForWidget(this._currentWidget);
-    if (!provider) {
-      console.warn(
-        'Unable to search on current widget, no compatible search provider'
-      );
-      return;
-    }
-    this._activeProvider = provider;
-    this._activeProvider.changed.connect(
-      this._onChanged,
-      this
-    );
-    return cleanupPromise.then(() =>
-      provider.startSearch(options, this._currentWidget)
-    );
-  }
-
-  endSearch(): Promise<void> {
-    if (!this._activeProvider) {
-      return Promise.resolve();
-    }
-    return this._activeProvider.endSearch().then(() => {
-      this._activeProvider.changed.disconnect(this._onChanged);
-      this._activeProvider = undefined;
-      this._currentWidget = undefined;
-    });
-  }
-
-  highlightNext(): Promise<ISearchMatch> {
-    if (!this._activeProvider) {
-      return Promise.resolve(null);
-    }
-    return this._activeProvider.highlightNext();
-  }
-
-  highlightPrevious(): Promise<ISearchMatch> {
-    if (!this._activeProvider) {
-      return Promise.resolve(null);
-    }
-    return this._activeProvider.highlightPrevious();
-  }
-
-  get currentMatchIndex(): number {
-    if (!this._activeProvider) {
-      return 0;
-    }
-    return this._activeProvider.currentMatchIndex;
-  }
-
-  get matches(): ISearchMatch[] {
-    if (!this._activeProvider) {
-      return [];
-    }
-    return this._activeProvider.matches;
-  }
-
-  get changed(): ISignal<this, void> {
-    return this._changed;
-  }
-
-  private _onChanged(): void {
-    this._changed.emit(null);
-  }
-  private _registry: SearchProviderRegistry;
-  private _activeProvider: ISearchProvider;
-  private _currentWidget: Widget;
-  private _changed: Signal<this, void> = new Signal<this, void>(this);
-}

packages/documentsearch-extension/src/index.ts

@@ -4,7 +4,6 @@
 |----------------------------------------------------------------------------*/
 import '../style/index.css';
 
-import { Executor } from './executor';
 import { SearchProviderRegistry } from './searchproviderregistry';
 
 import {
@@ -17,6 +16,7 @@ import { ICommandPalette } from '@jupyterlab/apputils';
 import { ISignal, Signal } from '@phosphor/signaling';
 import { createSearchOverlay } from './searchoverlay';
 import { Widget } from '@phosphor/widgets';
+import { DocumentWidget } from '@jupyterlab/docregistry';
 
 export interface ISearchMatch {
   /**
@@ -106,6 +106,8 @@ export interface IDisplayUpdate {
   inputText: string;
   query: RegExp;
   lastQuery: RegExp;
+  errorMessage: string;
+  forceFocus: boolean;
 }
 
 /**
@@ -161,65 +163,89 @@ const extension: JupyterLabPlugin<void> = {
 };
 
 class SearchInstance {
-  constructor(currentWidget: Widget, registry: SearchProviderRegistry) {
-    this._widget = currentWidget;
-    this.initializeSearchAssets(registry);
+  constructor(shell: ApplicationShell, registry: SearchProviderRegistry) {
+    this._widget = shell.currentWidget;
+    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() {
     return this._searchWidget;
   }
 
-  get executor() {
-    return this._executor;
+  get provider() {
+    return this._activeProvider;
+  }
+
+  focus(): void {
+    this._displayState.forceFocus = true;
+    this._displayUpdateSignal.emit(this._displayState);
   }
 
   updateIndices(): void {
-    this._displayState.totalMatches = this._executor.matches.length;
-    this._displayState.currentIndex = this._executor.currentMatchIndex;
+    this._displayState.totalMatches = this._activeProvider.matches.length;
+    this._displayState.currentIndex = this._activeProvider.currentMatchIndex;
     this.updateDisplay();
   }
 
   private _widget: Widget;
   private _displayState: IDisplayUpdate;
-  private _displayUpdateSignal: Signal<Executor, IDisplayUpdate>;
-  private _executor: Executor;
+  private _displayUpdateSignal: Signal<ISearchProvider, IDisplayUpdate>;
+  private _activeProvider: ISearchProvider;
   private _searchWidget: Widget;
   private updateDisplay() {
+    this._displayState.forceFocus = false;
     this._displayUpdateSignal.emit(this._displayState);
   }
   private startSearch(query: RegExp) {
     // save the last query (or set it to the current query if this is the first)
     this._displayState.lastQuery = this._displayState.query || query;
     this._displayState.query = query;
-    this._executor.startSearch(query).then(() => {
-      this.updateIndices();
-      // this signal should get injected when the widget is
-      // created and hooked up to react!
-      this._executor.changed.connect(
-        this.updateIndices,
-        this
-      );
-    });
+    let cleanupPromise = Promise.resolve();
+    if (this._activeProvider) {
+      cleanupPromise = this._activeProvider.endSearch();
+    }
+    cleanupPromise.then(() =>
+      this._activeProvider.startSearch(query, this._widget).then(() => {
+        this.updateIndices();
+        // this signal should get injected when the widget is
+        // created and hooked up to react!
+        this._activeProvider.changed.connect(
+          this.updateIndices,
+          this
+        );
+      })
+    );
   }
   private endSearch() {
-    this._executor.endSearch().then(() => {
+    this._activeProvider.endSearch().then(() => {
       // more cleanup probably
       Signal.disconnectAll(this);
       this._searchWidget.dispose();
-      this._executor.changed.disconnect(this.updateIndices, this);
+      this._activeProvider.changed.disconnect(this.updateIndices, this);
     });
   }
   private highlightNext() {
-    this._executor.highlightNext().then(this.updateIndices.bind(this));
+    this._activeProvider.highlightNext().then(this.updateIndices.bind(this));
   }
   private highlightPrevious() {
-    this._executor.highlightPrevious().then(this.updateIndices.bind(this));
+    this._activeProvider
+      .highlightPrevious()
+      .then(this.updateIndices.bind(this));
   }
-  private initializeSearchAssets(registry: SearchProviderRegistry) {
-    this._executor = new Executor(registry, this._widget);
-    this._displayUpdateSignal = new Signal<Executor, IDisplayUpdate>(
-      this._executor
+  private initializeSearchAssets(
+    registry: SearchProviderRegistry,
+    toolbarHeight: number
+  ) {
+    this._activeProvider = registry.getProviderForWidget(this._widget);
+    this._displayUpdateSignal = new Signal<ISearchProvider, IDisplayUpdate>(
+      this._activeProvider
     );
 
     this._displayState = {
@@ -229,7 +255,9 @@ class SearchInstance {
       useRegex: false,
       inputText: '',
       query: null,
-      lastQuery: null
+      lastQuery: null,
+      errorMessage: '',
+      forceFocus: true
     };
 
     const onCaseSensitiveToggled = () => {
@@ -250,7 +278,8 @@ class SearchInstance {
       this.highlightNext.bind(this),
       this.highlightPrevious.bind(this),
       this.startSearch.bind(this),
-      this.endSearch.bind(this)
+      this.endSearch.bind(this),
+      toolbarHeight
     );
   }
 }
@@ -283,12 +312,10 @@ namespace Private {
     const currentWidget = shell.currentWidget;
     const widgetId = currentWidget.id;
     if (activeSearches[widgetId]) {
-      const searchWidget = activeSearches[widgetId].searchWidget;
-      // searchWidget.focusInput(); // focus WAS TODO
-      searchWidget;
+      activeSearches[widgetId].focus();
       return;
     }
-    const searchInstance = new SearchInstance(currentWidget, registry);
+    const searchInstance = new SearchInstance(shell, registry);
     activeSearches[widgetId] = searchInstance;
 
     searchInstance.searchWidget.disposed.connect(() => {
@@ -298,11 +325,11 @@ namespace Private {
   }
 
   export function onNextCommand(instance: SearchInstance) {
-    instance.executor.highlightNext().then(() => instance.updateIndices());
+    instance.provider.highlightNext().then(() => instance.updateIndices());
   }
 
   export function onPrevCommand(instance: SearchInstance) {
-    instance.executor.highlightPrevious().then(() => instance.updateIndices());
+    instance.provider.highlightPrevious().then(() => instance.updateIndices());
   }
 }
 

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

@@ -20,20 +20,16 @@ export class CodeMirrorSearchProvider implements ISearchProvider {
     }
     Private.clearSearch(this._cm);
 
-    const state = Private.getSearchState(this._cm);
-    this._cm.operation(() => {
-      state.query = query;
-      // clear search first
-      this._cm.removeOverlay(state.overlay);
-      state.overlay = Private.searchOverlay(
-        state.query,
-        this._matchState,
-        this._changed
-      );
-      this._cm.addOverlay(state.overlay);
-      // skips show matches on scroll bar here
-      state.posFrom = state.posTo = this._cm.getCursor();
+    CodeMirror.on(this._cm.doc, 'change', (instance: any, changeObj: any) => {
+      // If we get newlines added/removed, the match state all goes out of whack
+      // so here we want to blow away the match state and re-do the search overlay
+      // to build a correct state for the codemirror instance.
+
+      if (changeObj.text.length > 1 || changeObj.removed.length > 1) {
+        this._refreshOverlay(query);
+      }
     });
+    this._refreshOverlay(query);
     const matches = Private.parseMatchesFromState(this._matchState);
     if (matches.length === 0) {
       return Promise.resolve([]);
@@ -56,7 +52,9 @@ export class CodeMirrorSearchProvider implements ISearchProvider {
   endSearch(): Promise<void> {
     this._matchState = {};
     this._matchIndex = 0;
-    Private.clearSearch(this._cm);
+    if (this._cm) {
+      Private.clearSearch(this._cm);
+    }
     return Promise.resolve();
   }
 
@@ -114,6 +112,25 @@ export class CodeMirrorSearchProvider implements ISearchProvider {
     return null;
   }
 
+  private _refreshOverlay(query: RegExp) {
+    this._matchState = {};
+    // multiple lines added
+    const state = Private.getSearchState(this._cm);
+    this._cm.operation(() => {
+      state.query = query;
+      // clear search first
+      this._cm.removeOverlay(state.overlay);
+      state.overlay = Private.searchOverlay(
+        state.query,
+        this._matchState,
+        this._changed
+      );
+      this._cm.addOverlay(state.overlay);
+      // skips show matches on scroll bar here
+      state.posFrom = state.posTo = this._cm.getCursor();
+      this._changed.emit(null);
+    });
+  }
   private _query: RegExp;
   private _cm: CodeMirrorEditor;
   private _matchIndex: number;
@@ -206,6 +223,7 @@ namespace Private {
     matchState: MatchMap,
     changed: Signal<ISearchProvider, void>
   ) {
+    let blankLineLast = false;
     return {
       /**
        * Token function is called when a line needs to be processed -
@@ -231,6 +249,10 @@ namespace Private {
           !!matchState[line] &&
           Object.keys(matchState[line]).length !== 0
         ) {
+          if (blankLineLast) {
+            matchState[line - 1] = {};
+            blankLineLast = false;
+          }
           matchState[line] = {};
         }
         if (match && match.index === currentPos) {

packages/documentsearch-extension/src/searchoverlay.tsx

@@ -4,8 +4,7 @@ import '../style/index.css';
 import { ReactWidget, UseSignal } from '@jupyterlab/apputils';
 import { Signal } from '@phosphor/signaling';
 import { Widget } from '@phosphor/widgets';
-import { Executor } from './executor';
-import { IDisplayUpdate } from '.';
+import { IDisplayUpdate, ISearchProvider } from '.';
 
 const OVERLAY_CLASS = 'jp-DocumentSearch-overlay';
 const INPUT_CLASS = 'jp-DocumentSearch-input';
@@ -16,6 +15,7 @@ const INDEX_COUNTER_CLASS = 'jp-DocumentSearch-index-counter';
 const UP_DOWN_BUTTON_WRAPPER_CLASS = 'jp-DocumentSearch-up-down-wrapper';
 const UP_DOWN_BUTTON_CLASS = 'jp-DocumentSearch-up-down-button-class';
 const CLOSE_BUTTON_CLASS = 'jp-DocumentSearch-close-button';
+const REGEX_ERROR_CLASS = 'jp-DocumentSearch-regex-error';
 
 interface ISearchEntryProps {
   onCaseSensitiveToggled: Function;
@@ -25,6 +25,7 @@ interface ISearchEntryProps {
   caseSensitive: boolean;
   useRegex: boolean;
   inputText: string;
+  forceFocus: boolean;
 }
 
 class SearchEntry extends React.Component<ISearchEntryProps> {
@@ -40,6 +41,12 @@ class SearchEntry extends React.Component<ISearchEntryProps> {
     (this.refs.searchInputNode as HTMLInputElement).focus();
   }
 
+  componentDidUpdate() {
+    if (this.props.forceFocus) {
+      this.focusInput();
+    }
+  }
+
   render() {
     const caseButtonToggleClass = this.props.caseSensitive
       ? INPUT_BUTTON_CLASS_ON
@@ -59,13 +66,13 @@ class SearchEntry extends React.Component<ISearchEntryProps> {
           ref="searchInputNode"
         />
         <button
-          className={`${caseButtonToggleClass}`}
+          className={caseButtonToggleClass}
           onClick={() => this.props.onCaseSensitiveToggled()}
         >
           A<sup>a</sup>
         </button>
         <button
-          className={`${regexButtonToggleClass}`}
+          className={regexButtonToggleClass}
           onClick={() => this.props.onRegexToggled()}
         >
           .*
@@ -144,13 +151,17 @@ class SearchOverlay extends React.Component<
       return;
     }
     // execute search!
-    const query: RegExp = Private.parseQuery(
-      this.state.inputText,
-      this.props.overlayState.caseSensitive,
-      this.props.overlayState.useRegex
-    );
-    if (!query) {
-      // display error! WAS TODO
+    let query;
+    try {
+      query = Private.parseQuery(
+        this.state.inputText,
+        this.props.overlayState.caseSensitive,
+        this.props.overlayState.useRegex
+      );
+      this.setState({ errorMessage: '' });
+    } catch (e) {
+      this.setState({ errorMessage: e.message });
+      return;
     }
     if (query.source.length === 0) {
       return;
@@ -184,6 +195,7 @@ class SearchOverlay extends React.Component<
           onKeydown={(e: KeyboardEvent) => this.onKeydown(e)}
           onChange={(e: React.ChangeEvent) => this.onChange(e)}
           inputText={this.state.inputText}
+          forceFocus={this.props.overlayState.forceFocus}
         />
         <SearchIndices
           currentIndex={this.props.overlayState.currentIndex}
@@ -194,20 +206,29 @@ class SearchOverlay extends React.Component<
           onHightlightNext={() => this.props.onHightlightNext()}
         />
         <div className={CLOSE_BUTTON_CLASS} onClick={() => this.onClose()} />
+        <div
+          className={REGEX_ERROR_CLASS}
+          hidden={
+            this.state.errorMessage && this.state.errorMessage.length === 0
+          }
+        >
+          {this.state.errorMessage}
+        </div>
       </div>
     );
   }
 }
 
 export function createSearchOverlay(
-  wigdetChanged: Signal<Executor, IDisplayUpdate>,
+  wigdetChanged: Signal<ISearchProvider, IDisplayUpdate>,
   overlayState: IDisplayUpdate,
   onCaseSensitiveToggled: Function,
   onRegexToggled: Function,
   onHightlightNext: Function,
   onHighlightPrevious: Function,
   onStartSearch: Function,
-  onEndSearch: Function
+  onEndSearch: Function,
+  toolbarHeight: number
 ): Widget {
   const widget = ReactWidget.create(
     <UseSignal signal={wigdetChanged} initialArgs={overlayState}>
@@ -227,6 +248,7 @@ export function createSearchOverlay(
     </UseSignal>
   );
   widget.addClass(OVERLAY_CLASS);
+  widget.node.style.top = toolbarHeight + 'px';
   return widget;
 }
 
@@ -241,12 +263,7 @@ namespace Private {
       ? queryString
       : queryString.replace(/[-[\]/{}()*+?.\\^$|]/g, '\\$&');
     let ret;
-    try {
-      ret = new RegExp(queryText, flag);
-    } catch (e) {
-      console.error('invalid regex: ', e);
-      return null;
-    }
+    ret = new RegExp(queryText, flag);
     if (ret.test('')) {
       ret = /x^/;
     }

packages/documentsearch-extension/style/index.css

@@ -1,19 +1,22 @@
 .jp-DocumentSearch-input {
   border: none;
   outline: none;
-  font-size: 16px;
+  font-size: var(--jp-ui-font-size1);
 }
 
 .jp-DocumentSearch-overlay {
   position: absolute;
-  background-color: var(--jp-layout-color2);
-  border: var(--jp-border-width) solid var(--jp-border-color0);
-  border-radius: var(--jp-border-radius);
+  background-color: var(--jp-layout-color0);
+  border-bottom: var(--jp-border-width) solid var(--jp-border-color0);
+  border-left: var(--jp-border-width) solid var(--jp-border-color0);
+  border-bottom-right-radius: var(--jp-border-radius);
+  border-bottom-left-radius: var(--jp-border-radius);
   top: 0;
   right: 0;
   z-index: 5;
-  min-width: 340px;
+  min-width: 300px;
   padding: 3px;
+  font-size: var(--jp-ui-font-size1);
 }
 
 .jp-DocumentSearch-overlay * {
@@ -86,3 +89,7 @@
   background-size: 16px;
   background-image: var(--jp-icon-close-circle);
 }
+
+.jp-DocumentSearch-regex-error {
+  color: var(--jp-error-color0);
+}