Input history was never adding dupes to the list (microsoft#4258)

* Add support for dupes showing up in history

* Add news entry

* Found another situation that didn't work.

* Fix hygiene

news/2 Fixes/4255.md

@@ -0,0 +1 @@
+More fixes for history in the Python Interactive window input prompt

src/datascience-ui/history-react/code.tsx

@@ -212,6 +212,7 @@ export class Code extends React.Component<ICodeProps, ICodeState> {
         // Double check we don't have an entirely empty document
         if (doc.getValue('').trim().length > 0) {
             let code = doc.getValue();
+            const isClean = doc.isClean();
             // We have to clear the history as this CodeMirror doesn't go away.
             doc.clearHistory();
             doc.setValue('');
@@ -223,7 +224,7 @@ export class Code extends React.Component<ICodeProps, ICodeState> {
 
             // Send to the input history too if necessary
             if (this.props.history) {
-                this.props.history.add(code);
+                this.props.history.add(code, !isClean);
             }
 
             this.props.onSubmit(code);
@@ -267,6 +268,7 @@ export class Code extends React.Component<ICodeProps, ICodeState> {
             if (newValue !== currentValue) {
                 doc.setValue(newValue);
                 doc.setCursor(0, doc.getLine(0).length);
+                doc.markClean();
             }
             return;
         }
@@ -282,6 +284,7 @@ export class Code extends React.Component<ICodeProps, ICodeState> {
             if (newValue !== currentValue) {
                 doc.setValue(newValue);
                 doc.setCursor(doc.lastLine(), doc.getLine(doc.lastLine()).length);
+                doc.markClean();
             }
             return;
         }

src/datascience-ui/history-react/inputHistory.ts

@@ -7,6 +7,7 @@ export class InputHistory {
     private historyStack: string [] = [];
     private up: number | undefined;
     private down: number | undefined;
+    private last: number | undefined;
 
     public completeUp(code: string) : string {
         // If going up, only move if anything in the history
@@ -34,14 +35,28 @@ export class InputHistory {
         return code;
     }
 
-    public add(code: string) {
-        // Compute our new history (dupes should reorder adding dupes)
-        const dupeIndex = this.historyStack.indexOf(code);
-        this.historyStack = dupeIndex >= 0 ? this.historyStack : [code, ...this.historyStack];
+    public add(code: string, typed: boolean) {
+        // Compute our new history. Behavior depends upon if the user typed it in or
+        // just used the arrows
 
-        // Reset position if new code
-        if (dupeIndex < 0) {
+        // Only skip adding a dupe if it's the same as the top item. Otherwise
+        // add it as normal.
+        this.historyStack = this.last === 0 && this.historyStack.length > 0 && this.historyStack[this.last] === code ?
+            this.historyStack : [code, ...this.historyStack];
+
+        // Position is more complicated. If we typed something start over
+        if (typed) {
             this.reset();
+        } else {
+            // We want our next up push to match the index of the item that was
+            // actually entered.
+            if (this.last === 0) {
+                this.up = undefined;
+                this.down = undefined;
+            } else {
+                this.up = this.last + 1;
+                this.down = this.last - 1;
+            }
         }
     }
 
@@ -51,6 +66,9 @@ export class InputHistory {
     }
 
     private adjustCursors(currentPos: number) {
+        // Save last position we entered.
+        this.last = currentPos;
+
         // For a single item, ony up works. But never modify it.
         if (this.historyStack.length > 1) {
             if (currentPos < this.historyStack.length) {

src/test/datascience/datascience.unit.test.ts

@@ -21,33 +21,33 @@ suite('Data Science Tests', () => {
 
     test('input history', async () => {
         let history = new InputHistory();
-        history.add('1');
-        history.add('2');
-        history.add('3');
-        history.add('4');
+        history.add('1', true);
+        history.add('2', true);
+        history.add('3', true);
+        history.add('4', true);
         assert.equal(history.completeDown('5'), '5');
-        history.add('5');
+        history.add('5', true);
         assert.equal(history.completeUp(''), '5');
-        history.add('5');
-        assert.equal(history.completeUp('5'), '4');
-        assert.equal(history.completeUp('4'), '3');
-        assert.equal(history.completeUp('2'), '2');
-        assert.equal(history.completeUp('1'), '1');
-        assert.equal(history.completeUp(''), '');
+        history.add('5', false);
+        assert.equal(history.completeUp('5'), '5');
+        assert.equal(history.completeUp('4'), '4');
+        assert.equal(history.completeUp('2'), '3');
+        assert.equal(history.completeUp('1'), '2');
+        assert.equal(history.completeUp(''), '1');
 
         // Add should reset position.
-        history.add('6');
+        history.add('6', true);
         assert.equal(history.completeUp(''), '6');
         assert.equal(history.completeUp(''), '5');
         assert.equal(history.completeUp(''), '4');
         assert.equal(history.completeUp(''), '3');
         assert.equal(history.completeUp(''), '2');
         assert.equal(history.completeUp(''), '1');
         history = new InputHistory();
-        history.add('1');
-        history.add('2');
-        history.add('3');
-        history.add('4');
+        history.add('1', true);
+        history.add('2', true);
+        history.add('3', true);
+        history.add('4', true);
         assert.equal(history.completeDown('5'), '5');
         assert.equal(history.completeDown(''), '');
         assert.equal(history.completeUp('1'), '4');
@@ -66,14 +66,21 @@ suite('Data Science Tests', () => {
         assert.equal(history.completeDown('2'), '3');
         assert.equal(history.completeDown('3'), '4');
         assert.equal(history.completeDown(''), '');
-        history.add('5');
+        history.add('5', true);
         assert.equal(history.completeUp('1'), '5');
         assert.equal(history.completeUp('1'), '4');
         assert.equal(history.completeUp('1'), '3');
+        history.add('3', false);
+        assert.equal(history.completeUp('1'), '3');
         assert.equal(history.completeUp('1'), '2');
         assert.equal(history.completeUp('1'), '1');
+        assert.equal(history.completeDown('1'), '2');
         assert.equal(history.completeUp('1'), '1');
         assert.equal(history.completeDown('1'), '2');
+        assert.equal(history.completeDown('1'), '3');
+        assert.equal(history.completeDown('1'), '4');
+        assert.equal(history.completeDown('1'), '5');
+        assert.equal(history.completeDown('1'), '3');
     });
 
 });