Merge pull request #6409 from ian-r-rose/more-granular-completion

More granular completion

packages/codemirror/src/editor.ts

@@ -867,12 +867,18 @@ export class CodeMirrorEditor implements CodeEditor.IEditor {
     switch (args.type) {
       case 'insert':
         let pos = doc.posFromIndex(args.start);
-        doc.replaceRange(args.value, pos, pos);
+        // Replace the range, including a '+input' orign,
+        // which indicates that CodeMirror may merge changes
+        // for undo/redo purposes.
+        doc.replaceRange(args.value, pos, pos, '+input');
         break;
       case 'remove':
         let from = doc.posFromIndex(args.start);
         let to = doc.posFromIndex(args.end);
-        doc.replaceRange('', from, to);
+        // Replace the range, including a '+input' orign,
+        // which indicates that CodeMirror may merge changes
+        // for undo/redo purposes.
+        doc.replaceRange('', from, to, '+input');
         break;
       case 'set':
         doc.setValue(args.value);

packages/completer/src/handler.ts

@@ -166,26 +166,22 @@ export class CompletionHandler implements IDisposable {
   /**
    * Handle a completion selected signal from the completion widget.
    */
-  protected onCompletionSelected(completer: Completer, value: string): void {
+  protected onCompletionSelected(completer: Completer, val: string): void {
     const model = completer.model;
     const editor = this._editor;
     if (!editor || !model) {
       return;
     }
 
-    const patch = model.createPatch(value);
+    const patch = model.createPatch(val);
 
     if (!patch) {
       return;
     }
 
-    const { offset, text } = patch;
-    editor.model.value.text = text;
-
-    const position = editor.getPositionAt(offset);
-    if (position) {
-      editor.setCursorPosition(position);
-    }
+    const { start, end, value } = patch;
+    editor.model.value.remove(start, end);
+    editor.model.value.insert(start, value);
   }
 
   /**

packages/completer/src/model.ts

@@ -96,7 +96,7 @@ export class CompleterModel implements Completer.IModel {
 
     // If the text change means that the original start point has been preceded,
     // then the completion is no longer valid and should be reset.
-    if (currentLine.length < originalLine.length) {
+    if (!this._subsetMatch && currentLine.length < originalLine.length) {
       this.reset(true);
       return;
     }
@@ -301,13 +301,6 @@ export class CompleterModel implements Completer.IModel {
       return;
     }
 
-    // When the completer detects a common subset prefix for all options,
-    // it updates the model and sets the model source to that value, but this
-    // text change should be ignored.
-    if (this._subsetMatch) {
-      return;
-    }
-
     const { text, column, line } = change;
     const last = text.split('\n')[line][column - 1];
 
@@ -319,7 +312,7 @@ export class CompleterModel implements Completer.IModel {
     }
 
     // If final character is whitespace, reset completion.
-    this.reset(true);
+    this.reset(false);
   }
 
   /**
@@ -337,12 +330,12 @@ export class CompleterModel implements Completer.IModel {
       return undefined;
     }
 
-    const { start, end } = cursor;
-    const { text } = original;
-    const prefix = text.substring(0, start);
-    const suffix = text.substring(end);
+    let { start, end } = cursor;
+    // Also include any filtering/additional-typing that has occurred
+    // since the completion request in the patched length.
+    end = end + (this.current.text.length - this.original.text.length);
 
-    return { offset: (prefix + patch).length, text: prefix + patch + suffix };
+    return { start, end, value: patch };
   }
 
   /**

packages/completer/src/widget.ts

@@ -1,6 +1,10 @@
 // Copyright (c) Jupyter Development Team.
 // Distributed under the terms of the Modified BSD License.
 
+import { HoverBox, defaultSanitizer } from '@jupyterlab/apputils';
+
+import { CodeEditor } from '@jupyterlab/codeeditor';
+
 import { IIterator, IterableOrArrayLike, toArray } from '@phosphor/algorithm';
 
 import { JSONObject, JSONExt } from '@phosphor/coreutils';
@@ -15,10 +19,6 @@ import { ISignal, Signal } from '@phosphor/signaling';
 
 import { Widget } from '@phosphor/widgets';
 
-import { HoverBox, defaultSanitizer } from '@jupyterlab/apputils';
-
-import { CodeEditor } from '@jupyterlab/codeeditor';
-
 /**
  * The class name added to completer menu items.
  */
@@ -662,14 +662,19 @@ export namespace Completer {
    */
   export interface IPatch {
     /**
-     * The patched text.
+     * The start of the range to be patched.
      */
-    text: string;
+    start: number;
+
+    /**
+     * The end of the range to be patched.
+     */
+    end: number;
 
     /**
-     * The offset of the cursor.
+     * The value to be patched in.
      */
-    offset: number;
+    value: string;
   }
 
   /**

tests/test-completer/src/handler.spec.ts

@@ -237,9 +237,11 @@ describe('@jupyterlab/completer', () => {
         const editor = createEditorWidget().editor;
         const text = 'eggs\nfoo # comment\nbaz';
         const want = 'eggs\nfoobar # comment\nbaz';
+        const line = 1;
+        const column = 5;
         const request: Completer.ITextState = {
-          column: 5,
-          line: 1,
+          column,
+          line,
           lineHeight: 0,
           charWidth: 0,
           coords: null,
@@ -248,10 +250,65 @@ describe('@jupyterlab/completer', () => {
 
         handler.editor = editor;
         handler.editor.model.value.text = text;
+        handler.editor.setCursorPosition({ line, column: column + 3 });
         model.original = request;
-        model.cursor = { start: 5, end: 8 };
+        model.cursor = { start: column, end: column + 3 };
         (completer.selected as any).emit(patch);
         expect(handler.editor.model.value.text).to.equal(want);
+        expect(handler.editor.getCursorPosition()).to.eql({
+          line,
+          column: column + 6
+        });
+      });
+
+      it('should be undoable and redoable', () => {
+        const model = new CompleterModel();
+        const patch = 'foobar';
+        const completer = new Completer({ editor: null, model });
+        const handler = new TestCompletionHandler({ completer, connector });
+        const editor = createEditorWidget().editor;
+        const text = 'eggs\nfoo # comment\nbaz';
+        const want = 'eggs\nfoobar # comment\nbaz';
+        const line = 1;
+        const column = 5;
+        const request: Completer.ITextState = {
+          column,
+          line,
+          lineHeight: 0,
+          charWidth: 0,
+          coords: null,
+          text
+        };
+
+        handler.editor = editor;
+        handler.editor.model.value.text = text;
+        handler.editor.setCursorPosition({ line, column: column + 3 });
+        model.original = request;
+        model.cursor = { start: column, end: column + 3 };
+        // Make the completion, check its value and cursor position.
+        (completer.selected as any).emit(patch);
+        expect(editor.model.value.text).to.equal(want);
+        expect(editor.getCursorPosition()).to.eql({
+          line,
+          column: column + 6
+        });
+        console.warn(editor.getCursorPosition());
+        // Undo the completion, check its value and cursor position.
+        editor.undo();
+        expect(editor.model.value.text).to.equal(text);
+        expect(editor.getCursorPosition()).to.eql({
+          line,
+          column: column + 3
+        });
+        console.warn(editor.getCursorPosition());
+        // Redo the completion, check its value and cursor position.
+        editor.redo();
+        expect(editor.model.value.text).to.equal(want);
+        expect(editor.getCursorPosition()).to.eql({
+          line,
+          column: column + 6
+        });
+        console.warn(editor.getCursorPosition());
       });
     });
   });

tests/test-completer/src/model.spec.ts

@@ -361,8 +361,9 @@ describe('completer/model', () => {
         let model = new CompleterModel();
         let patch = 'foobar';
         let want: Completer.IPatch = {
-          text: patch,
-          offset: patch.length
+          start: 0,
+          end: 3,
+          value: patch
         };
         let cursor: Completer.ICursorSpan = { start: 0, end: 3 };
         model.original = makeState('foo');
@@ -382,8 +383,9 @@ describe('completer/model', () => {
         let start = currentValue.length;
         let end = currentValue.length;
         let want: Completer.IPatch = {
-          text: currentValue + patch,
-          offset: currentValue.length + patch.length
+          start,
+          end,
+          value: patch
         };
         let cursor: Completer.ICursorSpan = { start, end };
         model.original = makeState(currentValue);