From f31b8b92d4324cbf71c3fec89139c6e40f959683 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 5 Jul 2017 12:24:45 +0000 Subject: [PATCH 01/19] Minor cleanup --- package.json | 69 +------------------- plugins/c9.ide.terminal/predict_echo.js | 24 ++----- plugins/c9.ide.terminal/predict_echo_test.js | 5 +- 3 files changed, 10 insertions(+), 88 deletions(-) diff --git a/package.json b/package.json index bbacb9eb..0c5aea32 100644 --- a/package.json +++ b/package.json @@ -56,72 +56,5 @@ "architect-build", "msgpack-js", "c9" - ], - "c9plugins": { - "c9.ide.language": "#9843fc4875", - "c9.ide.language.core": "#208159eda7", - "c9.ide.language.css": "#5827caacda", - "c9.ide.language.generic": "#809e6994e6", - "c9.ide.language.html": "#ae00c111da", - "c9.ide.language.html.diff": "#33ac0f78aa", - "c9.ide.language.javascript": "#6f86fc7900", - "c9.ide.language.javascript.immediate": "#efc3518d0a", - "c9.ide.language.javascript.eslint": "#004c86f8ef", - "c9.ide.language.javascript.tern": "#cc569cfa6f", - "c9.ide.language.javascript.infer": "#d9f6dc5e51", - "c9.ide.language.jsonalyzer": "#6d0ed9fbf6", - "c9.ide.language.codeintel": "#435e084066", - "c9.ide.collab": "#5a2071f4ea", - "c9.ide.local": "#976ad9fca0", - "c9.ide.find": "#5c1d1ea078", - "c9.ide.find.infiles": "#d4762d1382", - "c9.ide.find.replace": "#1f96b87bf7", - "c9.ide.run.debug": "#91bc33714f", - "c9.automate": "#1c6006046e", - "c9.ide.ace.emmet": "#06791647c5", - "c9.ide.ace.gotoline": "#636d426035", - "c9.ide.ace.keymaps": "#832302482c", - "c9.ide.ace.repl": "#623561ecff", - "c9.ide.ace.split": "#d2143d8336", - "c9.ide.ace.statusbar": "#102340271c", - "c9.ide.ace.stripws": "#0aaa293283", - "c9.ide.behaviors": "#cc04e94c62", - "c9.ide.closeconfirmation": "#6b758eb957", - "c9.ide.configuration": "#5d8c381035", - "c9.ide.dialog.wizard": "#c8200c6ad1", - "c9.ide.fontawesome": "#96cb342a4a", - "c9.ide.format": "#660cf065ac", - "c9.ide.help.support": "#42d843e0db", - "c9.ide.imgeditor": "#353308ec1b", - "c9.ide.immediate": "#909e6f0bd1", - "c9.ide.installer": "#5f8f75eb2c", - "c9.ide.language.python": "#102398b335", - "c9.ide.language.go": "#e018761e24", - "c9.ide.navigate": "#992381bbf6", - "c9.ide.newresource": "#8ce79bb66c", - "c9.ide.openfiles": "#8ab1e2c85f", - "c9.ide.preview": "#a337636943", - "c9.ide.preview.browser": "#339cc3c10d", - "c9.ide.preview.markdown": "#df4b12c34c", - "c9.ide.pubsub": "#2accad6c0f", - "c9.ide.readonly": "#59bd0c4961", - "c9.ide.recentfiles": "#e8a52f2015", - "c9.ide.remote": "#14f717a0d5", - "c9.ide.processlist": "#99111d40fd", - "c9.ide.run": "#9dfbcebf81", - "c9.ide.run.build": "#652cd18587", - "c9.ide.run.debug.xdebug": "#def07a584b", - "c9.ide.run.debug.ikpdb": "#a223b727e0", - "c9.ide.save": "#9b06b29d7e", - "c9.ide.scm": "#a2dd947ed5", - "c9.ide.terminal.monitor": "#9dfda63972", - "c9.ide.test": "#4a56c01f66", - "c9.ide.test.mocha": "#ceace59aaf", - "c9.ide.theme.flat": "#6f9382edeb", - "c9.ide.threewaymerge": "#7a3a0dd1ad", - "c9.ide.undo": "#0e47e11192", - "c9.ide.upload": "#7ceff1a51d", - "c9.ide.welcome": "#b352446b17", - "c9.ide.guide": "#4249f58769" - } + ] } \ No newline at end of file diff --git a/plugins/c9.ide.terminal/predict_echo.js b/plugins/c9.ide.terminal/predict_echo.js index 44e40ecf..864d6356 100644 --- a/plugins/c9.ide.terminal/predict_echo.js +++ b/plugins/c9.ide.terminal/predict_echo.js @@ -152,7 +152,7 @@ define(function(require, exports, module) { command.after = { predict: predictLine, predictIndex: predictIndex }; command.sent = Date.now(); - DEBUG && console.log("!" + DEBUG && console.log("! " + nonPredictTerminal.$debugCharsAt(nonPredictTerminal.y) .slice(0, predictStartX) .map(function(c) { return c || " "; }) @@ -347,14 +347,14 @@ define(function(require, exports, module) { * @param {Object[]} predictions * @param {Function} callback * @param {Error} callback.err - * @param {Boolean} callback.result Whether prediction was succesful. + * @param {Object[]|Boolean} callback.results A list of matching predictions, or `false` */ function chopPredictions(e, predictions, callback) { var line = nonPredictTerminal.lines[nonPredictStartY]; var rowChanged = nonPredictStartY !== nonPredictTerminal.y + nonPredictTerminal.ybase; if (!checkTextBeforePrediction()) - return callback(null, false); + return done(null, false); // Check if predictions became true var matchedOneOff = false; @@ -600,12 +600,10 @@ define(function(require, exports, module) { }; function BackspaceCommand() { var after = predictLine.substr(predictIndex); - var deletedChar; var outputText = OUTPUTS_BACKSPACE_CHAR[0]; return { $outputText: outputText, do: function() { - deletedChar = peek(-1); predictLine = predictLine.substr(0, predictIndex - 1) + after; predictIndex--; echo(outputText); @@ -622,11 +620,9 @@ define(function(require, exports, module) { }; function DeleteCommand() { var after = predictLine.substr(predictIndex + 1); - var deletedChar; return { $outputText: OUTPUTS_DELETE_CHAR[0], do: function() { - deletedChar = peek(); predictLine = predictLine.substr(0, predictIndex) + after; echo(OUTPUTS_DELETE_CHAR[0]); } @@ -641,12 +637,10 @@ define(function(require, exports, module) { return new CursorLeftCommand(inputText); }; function CursorLeftCommand() { - var noChange = false; return { $outputText: OUTPUTS_LEFT[0], do: function() { if (predictIndex === 0) { - noChange = true; clearTimeout(this.timeout); return; } @@ -683,12 +677,10 @@ define(function(require, exports, module) { return new HomeCommand(inputText); }; function HomeCommand() { - var oldIndex; var outputText = predictIndex ? getCursorLeft(predictIndex) : ""; return { $outputText: outputText, do: function() { - oldIndex = predictIndex; echo(outputText); predictIndex = 0; } @@ -712,14 +704,8 @@ define(function(require, exports, module) { }; } - function charsOf(s) { - var r1 = []; - var r2 = []; - for (var i = 0; i < s.length; i++) { - r1.push(s.charAt(i)); - r2.push(s.charCodeAt(i)); - } - return [r1, r2]; + function charsOf(line) { + return line.map(function(c) { return c[1] }).join(""); } function getCursorLeft(n) { diff --git a/plugins/c9.ide.terminal/predict_echo_test.js b/plugins/c9.ide.terminal/predict_echo_test.js index 4213e66f..cffba81d 100644 --- a/plugins/c9.ide.terminal/predict_echo_test.js +++ b/plugins/c9.ide.terminal/predict_echo_test.js @@ -128,8 +128,10 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse }); }); - function openTerminal(done) { + function openTerminal(callback) { tabs.openEditor("terminal", function(err, tab) { + if (err) return callback(err); + editor = tab.editor; session = editor.ace.getSession().c9session; send = session.send; @@ -219,6 +221,7 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse send("\r"); }); session.$predictor.state = 0; + console.log("! next test"); sendAll(" # next*".split("")); }); From 726a40fb87ee52738ec611ea1193d696ade272ff Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 5 Jul 2017 14:39:25 +0000 Subject: [PATCH 02/19] Make sure we always send the "predict" event --- plugins/c9.ide.terminal/predict_echo.js | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/plugins/c9.ide.terminal/predict_echo.js b/plugins/c9.ide.terminal/predict_echo.js index 864d6356..a14d5933 100644 --- a/plugins/c9.ide.terminal/predict_echo.js +++ b/plugins/c9.ide.terminal/predict_echo.js @@ -390,26 +390,32 @@ define(function(require, exports, module) { predict = predictions.splice(0, i + 1); } - emit("predict", { - data: e.data, - session: session, - predictions: predict - }); - return callback(null, predict); + return done(null, predict); } } // No matches. But one got really close. if (matchedOneOff) - return callback(null, []); + return done(null, []); // No matches. Return if our predictions were optional. if (isOptionalOnly(predictions)) - return callback(null, []); + return done(null, []); // No matches for our predictions :( We likely made a mistake. // Reporting false here ensures we catch mistakes early. - return callback(null, false); + return done(null, false); + + function done(err, result) { + if (result) { + emit("predict", { + data: e.data, + session: session, + predictions: predict + }); + } + callback(err, result, line); + } function matchPrediction(prediction) { var predict = prediction.after.predict; From bfd5aa7f44be8490097a6c251e5a502417c50bb9 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 5 Jul 2017 14:41:01 +0000 Subject: [PATCH 03/19] Improve debuggability --- plugins/c9.ide.terminal/predict_echo.js | 23 +++++++++++++------- plugins/c9.ide.terminal/predict_echo_test.js | 15 +++++++------ 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/plugins/c9.ide.terminal/predict_echo.js b/plugins/c9.ide.terminal/predict_echo.js index a14d5933..6e40b44b 100644 --- a/plugins/c9.ide.terminal/predict_echo.js +++ b/plugins/c9.ide.terminal/predict_echo.js @@ -160,6 +160,9 @@ define(function(require, exports, module) { + "%c" + predictLine, "color: lightblue"); + // DEBUG && console.log("!=" + // + session.terminal.$debugCharsAt(predictStartY - session.terminal.ybase).join("")); + command.timeout = setTimeout(function panic() { if (!c9.has(c9.NETWORK) || !c9.connected) { state = STATE_WAIT_FOR_ECHO; @@ -219,13 +222,17 @@ define(function(require, exports, module) { pong(); - var result; - chopPredictions(e, predictions, function(err, _result) { - result = _result; - if (err || !result) { - DEBUG && console.log("[predict_echo] mispredict?", e.data.replace(/\r/g, "\\r"), - "\n@", nonPredictTerminal.$debugCharsAt(e.$startY).join("")); - emit("mispredict", { data: e.data, predictions: predictions, session: session }); + chopPredictions(e, predictions, function(err, results, line) { + if (err || !results) { + DEBUG && console.log("[predict_echo] mispredict?", e.data.replace(/\r/g, "\\r") + + "\n!=" + session.terminal.$debugCharsAt(predictStartY - session.terminal.ybase).join("") + + "\n<=" + nonPredictTerminal.$debugCharsAt(e.$startY).join("")); + emit("mispredict", { + data: e.data, + line: charsOf(line), + predictions: predictions, + session: session + }); undoPredictions(); } // I would try to enable predictions here, @@ -244,7 +251,7 @@ define(function(require, exports, module) { /** * Temporarily restore the unpredict terminal state to allow * writing incoming data, including small anomalies that may - * not have been predict but still passed our sanity checks. + * not have been predicted but still passed our sanity checks. */ function writePredictData(data, startX) { var predictTerminal = session.terminal; diff --git a/plugins/c9.ide.terminal/predict_echo_test.js b/plugins/c9.ide.terminal/predict_echo_test.js index cffba81d..461407f1 100644 --- a/plugins/c9.ide.terminal/predict_echo_test.js +++ b/plugins/c9.ide.terminal/predict_echo_test.js @@ -376,8 +376,8 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse afterPredict("[", function() { afterPredict("[", function() { assert.equal(peek(-1), " "); - assert.equal(peek(), "e"); - assert.equal(peek(1), "c"); + assert.equal(peek(), "#"); + assert.equal(peek(1), "p"); afterPrompt(done); send("\r"); @@ -387,16 +387,16 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse sendAll([INPUT_RIGHT]); }); - sendAll(["eecho bleep", INPUT_HOME]); + sendAll(["##print some chars; home; right; backspace", INPUT_HOME]); }); - it("supports insert with repeated characters; stress test", function loop(done, attempt) { + it("supports insert with repeated characters (prxaat); stress test", function loop(done, attempt) { this.timeout && this.timeout(60000); session.$predictor.state = 0; if (attempt === 5) return done(); - sendAll("echo blaat".split(""), function() { + sendAll("echo praat".split(""), function() { var sawX; afterPredict("t", function() { @@ -405,8 +405,9 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse }); predictor.on("predict", function wait(e) { sawX = sawX || e.data.match(/x/); - if (!sawX || e.data.match(/xaat/) || !e.data.match(/a/)) - return; // console.log(" -", e.data, sawX)* + // Wait until we've seen an 'x' and then an 'a' + if (!sawX || e.data.match(/xaat$/)) + return console.log(" -", e.data, !!sawX); predictor.off("predict", wait); assert.equal(peek(), "a"); From a734df504e84b5910edd061d7ce756fced38bdf7 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 5 Jul 2017 14:41:15 +0000 Subject: [PATCH 04/19] Consider match success if input seemed to be a noop --- plugins/c9.ide.terminal/predict_echo.js | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/plugins/c9.ide.terminal/predict_echo.js b/plugins/c9.ide.terminal/predict_echo.js index 6e40b44b..00be29f0 100644 --- a/plugins/c9.ide.terminal/predict_echo.js +++ b/plugins/c9.ide.terminal/predict_echo.js @@ -408,6 +408,10 @@ define(function(require, exports, module) { // No matches. Return if our predictions were optional. if (isOptionalOnly(predictions)) return done(null, []); + + // No matches. But it seems we got a noop input. Our predictions likely happen later. + if (matchPrediction(new NoopCommand())) + return done(null, []); // No matches for our predictions :( We likely made a mistake. // Reporting false here ensures we catch mistakes early. @@ -700,6 +704,26 @@ define(function(require, exports, module) { }; } + /** + * Noop command. Factory method: tryCreate(). + */ + NoopCommand.tryCreate = function() { + var result = new NoopCommand(); + result.before = { predict: predictLine, predictIndex: predictIndex }; + result.after = { predict: predictLine, predictIndex: predictIndex }; + return result; + }; + function NoopCommand() { + var outputText = predictIndex ? getCursorLeft(predictIndex) : ""; + return { + $outputText: outputText, + do: function() { + echo(outputText); + predictIndex = 0; + } + }; + } + // Predictor API return { get state() { return state; }, From 7284374f93b9f34eca62ce6367d37a95084e3b63 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 5 Jul 2017 14:42:38 +0000 Subject: [PATCH 05/19] Only report mispredictions during tests --- plugins/c9.ide.terminal/predict_echo_test.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/plugins/c9.ide.terminal/predict_echo_test.js b/plugins/c9.ide.terminal/predict_echo_test.js index 461407f1..d0509bfa 100644 --- a/plugins/c9.ide.terminal/predict_echo_test.js +++ b/plugins/c9.ide.terminal/predict_echo_test.js @@ -139,7 +139,7 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse setTimeout(init); function init() { - afterPrompt(function() { setTimeout(start); }); + afterPrompt(function() { setTimeout(callback); }); // Make sure we have a prompt with a dollar for tests // And terminal won't send rename commands in the middle of the test // TODO: do we need to handle rename sequence in predict_echo instead? @@ -150,14 +150,12 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse // editor.ace.onTextInput("ssh ubuntu@ci.c9.io\n"); } }); - function start() { - predictor.on("mispredict", function(e) { - console.error("MISPREDICTED", e); - delete e.session; - throw new Error("MISPREDICTED: " + JSON.stringify(e)); - }); - setTimeout(done); - } + } + + function reportMispredict(e) { + console.error("MISPREDICTED", e); + delete e.session; + throw new Error("MISPREDICTED: " + JSON.stringify(e)); } function peek(offset) { @@ -213,9 +211,12 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse describe("predict_echo", function() { beforeEach(function(done) { + predictor.off("mispredict", reportMispredict); + afterPredict("*", function() { afterPrompt(function() { session.$predictor.state = 0; + predictor.on("mispredict", reportMispredict); done(); }); send("\r"); From de4ceef6880cd49c4186bd4361bebd082fd9ce41 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 5 Jul 2017 14:43:14 +0000 Subject: [PATCH 06/19] Add small delay between keypresses --- plugins/c9.ide.terminal/predict_echo_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/c9.ide.terminal/predict_echo_test.js b/plugins/c9.ide.terminal/predict_echo_test.js index d0509bfa..9c67164b 100644 --- a/plugins/c9.ide.terminal/predict_echo_test.js +++ b/plugins/c9.ide.terminal/predict_echo_test.js @@ -206,7 +206,7 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse setTimeout(function() { send(key); sendAll(keys, callback); - }); + }, 5); } describe("predict_echo", function() { From 5356f9c0b24869f76027711a7dd1930508136161 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 5 Jul 2017 14:49:03 +0000 Subject: [PATCH 07/19] Correctly create object --- plugins/c9.ide.terminal/predict_echo.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/c9.ide.terminal/predict_echo.js b/plugins/c9.ide.terminal/predict_echo.js index 00be29f0..9f99b303 100644 --- a/plugins/c9.ide.terminal/predict_echo.js +++ b/plugins/c9.ide.terminal/predict_echo.js @@ -410,7 +410,7 @@ define(function(require, exports, module) { return done(null, []); // No matches. But it seems we got a noop input. Our predictions likely happen later. - if (matchPrediction(new NoopCommand())) + if (matchPrediction(NoopCommand.tryCreate())) return done(null, []); // No matches for our predictions :( We likely made a mistake. From 3804feb7b99e3bc81aea6edcb04dfce9bed06120 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 5 Jul 2017 19:30:20 +0000 Subject: [PATCH 08/19] Eliminate needless "wait for only echoes" state --- plugins/c9.ide.terminal/predict_echo.js | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/plugins/c9.ide.terminal/predict_echo.js b/plugins/c9.ide.terminal/predict_echo.js index 9f99b303..d17d3905 100644 --- a/plugins/c9.ide.terminal/predict_echo.js +++ b/plugins/c9.ide.terminal/predict_echo.js @@ -45,10 +45,9 @@ define(function(require, exports, module) { var INPUT_RIGHT = ESC + "[C"; var OUTPUTS_RIGHT = [ESC + "[C", ESC + "[1C"]; var STATE_PREDICT = 0; - var STATE_WAIT_FOR_ECHO_OR_PROMPT = 1; - var STATE_WAIT_FOR_ECHO = 2; - var STATE_WAIT_FOR_PROMPT = 3; - var STATE_INITING = 4; + var STATE_WAIT_FOR_PROMPT_OR_ECHO = 1; + var STATE_WAIT_FOR_PROMPT = 2; + var STATE_INITING = 3; var plugin = new Plugin("Ajax.org", main.consumes); var emit = plugin.getEmitter(); @@ -86,7 +85,7 @@ define(function(require, exports, module) { var predictStartX = 0; var predictStartY = 0; var nonPredictStartY = 0; - var state = STATE_WAIT_FOR_ECHO_OR_PROMPT; + var state = STATE_WAIT_FOR_PROMPT_OR_ECHO; var lastInput = null; // We maintain a copy of the terminal state without predictions @@ -117,7 +116,7 @@ define(function(require, exports, module) { if (isPossibleConnectionGone()) { DEBUG && console.log("!", "nopredict: connection gone?"); - state = STATE_WAIT_FOR_ECHO; + state = STATE_WAIT_FOR_PROMPT_OR_ECHO; return; } @@ -165,7 +164,7 @@ define(function(require, exports, module) { command.timeout = setTimeout(function panic() { if (!c9.has(c9.NETWORK) || !c9.connected) { - state = STATE_WAIT_FOR_ECHO; + state = STATE_WAIT_FOR_PROMPT_OR_ECHO; c9.once("connect", function() { command.timeout = setTimeout(panic, MIN_PREDICTION_WAIT); }); @@ -213,7 +212,7 @@ define(function(require, exports, module) { if (!predictions.length) { if (state == STATE_PREDICT && nonPredictStartY !== nonPredictTerminal.ybase + nonPredictTerminal.y) { DEBUG && console.log(" ^ disabled predictions: (row changed)"); - state = STATE_WAIT_FOR_ECHO; + state = STATE_WAIT_FOR_PROMPT_OR_ECHO; } tryEnablePrediction(e.data); emit("nopredict", { data: e.data, session: session }); @@ -295,7 +294,7 @@ define(function(require, exports, module) { pendingPings = []; predictions = []; - state = STATE_WAIT_FOR_ECHO; + state = STATE_WAIT_FOR_PROMPT_OR_ECHO; copyTerminalLineTo(terminal); session.terminal.x = nonPredictTerminal.x; lastInput = null; // avoid immediately enabling again @@ -483,7 +482,7 @@ define(function(require, exports, module) { return; // Enable prediction when we see a prompt - if ((state == STATE_WAIT_FOR_PROMPT || state === STATE_WAIT_FOR_ECHO_OR_PROMPT) + if ((state == STATE_WAIT_FOR_PROMPT || state === STATE_WAIT_FOR_PROMPT_OR_ECHO) && data.match(/[$#] $/)) { if (DEBUG) console.log(" ^ re-enabled predictions: (prompt)"); return startPredict(); @@ -491,7 +490,7 @@ define(function(require, exports, module) { // Enable prediction when we see echoing if (lastInput - && (state === STATE_WAIT_FOR_ECHO || state === STATE_WAIT_FOR_ECHO_OR_PROMPT) + && (state === STATE_WAIT_FOR_PROMPT_OR_ECHO) && lastInput === data.substr(data.length - lastInput.length) && (!BASH_ONLY || isBashActive())) { if (DEBUG) console.log(" ^ re-enabled predictions:", lastInput); From 50b131f1561194116ca8594363fee4a0d1c79af5 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 5 Jul 2017 19:32:02 +0000 Subject: [PATCH 09/19] Fix initialization with multiple terminals --- plugins/c9.ide.terminal/predict_echo.js | 33 ++++++++++++++++--------- 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/plugins/c9.ide.terminal/predict_echo.js b/plugins/c9.ide.terminal/predict_echo.js index d17d3905..d52ff1e7 100644 --- a/plugins/c9.ide.terminal/predict_echo.js +++ b/plugins/c9.ide.terminal/predict_echo.js @@ -71,6 +71,9 @@ define(function(require, exports, module) { terminal.on("beforeWrite", function(e) { return e.session.$predictor.onBeforeWrite(e); }, plugin); + terminal.on("afterWrite", function(e) { + return e.session.$predictor.onAfterWrite(e); + }, plugin); terminal.on("input", function(e) { DEBUG && console.log(">", e.data.replace("\r", "\\r").replace("\u007F", "\\bs")); return e.session.$predictor.onInput(e); @@ -492,28 +495,33 @@ define(function(require, exports, module) { if (lastInput && (state === STATE_WAIT_FOR_PROMPT_OR_ECHO) && lastInput === data.substr(data.length - lastInput.length) - && (!BASH_ONLY || isBashActive())) { + && (!BASH_ONLY || isBashActive()) + && checkTextBeforePrediction()) { if (DEBUG) console.log(" ^ re-enabled predictions:", lastInput); return startPredict(); } } function startPredict() { - state = STATE_INITING; predictIndex = 0; predictLine = ""; predictStartX = nonPredictTerminal.x; nonPredictStartY = nonPredictTerminal.y + nonPredictTerminal.ybase; predictStartY = session.terminal.y + session.terminal.ybase; - terminal.once("afterWrite", function() { - predictStartY = session.terminal.y + session.terminal.ybase; - state = STATE_PREDICT; - if (!checkTextBeforePrediction()) { - // Appears to happen when tmux or shell unexpectedly sends a new line - console.warn("Unable to init predictions"); - state = STATE_WAIT_FOR_ECHO; - } - }); + state = STATE_INITING; + } + + function onAfterWrite(e) { + if (state !== STATE_INITING) + return; + + predictStartY = session.terminal.y + session.terminal.ybase; + state = STATE_PREDICT; + if (!checkTextBeforePrediction()) { + // Appears to happen when tmux or shell unexpectedly sends a new line + console.log("[predict_echo] Unable to init predictions; will try again later"); + state = STATE_WAIT_FOR_PROMPT; + } } function isBashActive() { @@ -736,7 +744,8 @@ define(function(require, exports, module) { get predictions() { return predictions; }, undoPredictions: undoPredictions, onInput: onInput, - onBeforeWrite: onBeforeWrite + onBeforeWrite: onBeforeWrite, + onAfterWrite: onAfterWrite, }; } From 2f338e285575435d8b1f20ab7cdf810b89c20fe6 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 5 Jul 2017 19:52:21 +0000 Subject: [PATCH 10/19] Fix constants in tests --- plugins/c9.ide.terminal/predict_echo_test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/c9.ide.terminal/predict_echo_test.js b/plugins/c9.ide.terminal/predict_echo_test.js index 9c67164b..e278a713 100644 --- a/plugins/c9.ide.terminal/predict_echo_test.js +++ b/plugins/c9.ide.terminal/predict_echo_test.js @@ -91,9 +91,9 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse var INPUT_DELETE = ESC + "[3~"; var OUTPUT_BACKSPACE = "\b" + ESC + "[K"; var OUTPUT_DELETE_CHAR = ESC + "[P"; - var STATE_WAIT_FOR_ECHO_OR_PROMPT = 1; - var STATE_WAIT_FOR_ECHO = 2; - var STATE_WAIT_FOR_PROMPT = 3; + var STATE_WAIT_FOR_PROMPT_OR_ECHO = 1; + var STATE_WAIT_FOR_PROMPT = 2; + var STATE_INITING = 3; expect.html.setConstructor(function(tab) { if (typeof tab == "object") @@ -499,11 +499,11 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse }); // sometimes backspace will re-enable state 0; we reset it here - session.$predictor.state = STATE_WAIT_FOR_ECHO_OR_PROMPT; + session.$predictor.state = STATE_WAIT_FOR_PROMPT_OR_ECHO; send(":"); }); - session.$predictor.state = STATE_WAIT_FOR_ECHO_OR_PROMPT; + session.$predictor.state = STATE_WAIT_FOR_PROMPT_OR_ECHO; send(INPUT_BACKSPACE); }); From ff501f79f8b18d140930b897d5bd3fb9b394c428 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 5 Jul 2017 19:52:32 +0000 Subject: [PATCH 11/19] Enable prediction after backspace on prompt --- plugins/c9.ide.terminal/predict_echo.js | 14 ++++++++++++-- plugins/c9.ide.terminal/predict_echo_test.js | 12 ++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/plugins/c9.ide.terminal/predict_echo.js b/plugins/c9.ide.terminal/predict_echo.js index d52ff1e7..867048db 100644 --- a/plugins/c9.ide.terminal/predict_echo.js +++ b/plugins/c9.ide.terminal/predict_echo.js @@ -495,8 +495,18 @@ define(function(require, exports, module) { if (lastInput && (state === STATE_WAIT_FOR_PROMPT_OR_ECHO) && lastInput === data.substr(data.length - lastInput.length) - && (!BASH_ONLY || isBashActive()) - && checkTextBeforePrediction()) { + && checkTextBeforePrediction() + && (!BASH_ONLY || isBashActive())) { + if (DEBUG) console.log(" ^ re-enabled predictions:", lastInput); + return startPredict(); + } + + // Enable predictions when we see echoing *and* a prompt + if (lastInput + && state == STATE_WAIT_FOR_PROMPT + && lastInput === data.substr(data.length - lastInput.length) + && checkTextBeforePrediction() + && isBashActive()) { if (DEBUG) console.log(" ^ re-enabled predictions:", lastInput); return startPredict(); } diff --git a/plugins/c9.ide.terminal/predict_echo_test.js b/plugins/c9.ide.terminal/predict_echo_test.js index e278a713..3af90267 100644 --- a/plugins/c9.ide.terminal/predict_echo_test.js +++ b/plugins/c9.ide.terminal/predict_echo_test.js @@ -547,6 +547,18 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse }); }); + + it("recovers after spurious backspaces on a prompt", function(done) { + predictor.once("nopredict", function() { + predictor.once("nopredict", function() { + assert.equal(session.$predictor.state, STATE_INITING); + afterPrompt(done); + send("\r"); + }); + send("Q"); + }); + send(INPUT_BACKSPACE); + }); }); if (!onload.remain) { From 96b85d3ee148c4b285455227b951b905fa8f95e2 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 5 Jul 2017 20:35:16 +0000 Subject: [PATCH 12/19] Improve debug output --- plugins/c9.ide.terminal/predict_echo.js | 29 ++++++++++++++------ plugins/c9.ide.terminal/predict_echo_test.js | 2 +- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/plugins/c9.ide.terminal/predict_echo.js b/plugins/c9.ide.terminal/predict_echo.js index 867048db..e1b33f6c 100644 --- a/plugins/c9.ide.terminal/predict_echo.js +++ b/plugins/c9.ide.terminal/predict_echo.js @@ -154,13 +154,14 @@ define(function(require, exports, module) { command.after = { predict: predictLine, predictIndex: predictIndex }; command.sent = Date.now(); - DEBUG && console.log("! " - + nonPredictTerminal.$debugCharsAt(nonPredictTerminal.y) - .slice(0, predictStartX) - .map(function(c) { return c || " "; }) - .join("") - + "%c" + predictLine, - "color: lightblue"); + if (DEBUG) { + var alreadyEchoed = predictions[0].before.predict; + console.log("! " + debugPromptSuffix() + + predictLine.substr(0, alreadyEchoed.length) + + "%c" + predictLine.substr(alreadyEchoed.length), + "color: lightblue" + ); + } // DEBUG && console.log("!=" // + session.terminal.$debugCharsAt(predictStartY - session.terminal.ybase).join("")); @@ -189,6 +190,12 @@ define(function(require, exports, module) { } } + function debugPromptSuffix() { + return nonPredictTerminal.$debugCharsAt(nonPredictTerminal.y) + .slice(0, predictStartX).slice(-3) + .map(function(c) { return c || " "; }).join(""); + } + function isPossibleConnectionGone() { if (!pendingPings.length) return; @@ -208,8 +215,12 @@ define(function(require, exports, module) { DEBUG && console.log( "< " - + (state == STATE_PREDICT ? nonPredictTerminal.$debugCharsAt(e.$startY).join("") + " < " : "") - + e.data + + (state == STATE_PREDICT + ? debugPromptSuffix() + + nonPredictTerminal.$debugCharsAt(e.$startY).slice(predictStartX).join("") + : "") + + "%c < " + e.data, + "color: lightblue" ); if (!predictions.length) { diff --git a/plugins/c9.ide.terminal/predict_echo_test.js b/plugins/c9.ide.terminal/predict_echo_test.js index 3af90267..8b9e2834 100644 --- a/plugins/c9.ide.terminal/predict_echo_test.js +++ b/plugins/c9.ide.terminal/predict_echo_test.js @@ -143,7 +143,7 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse // Make sure we have a prompt with a dollar for tests // And terminal won't send rename commands in the middle of the test // TODO: do we need to handle rename sequence in predict_echo instead? - editor.ace.onTextInput("PS1='. $ ';" + editor.ace.onTextInput("PS1='P$ ';" + "tmux setw automatic-rename off;" + "printf '\\x1b]0;predict echo\\x07'\n"); // editor.ace.onTextInput("ssh lennart\n"); From 58cbafc603221c3b5bde97b3020df5ab2de34597 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Thu, 6 Jul 2017 07:15:01 +0000 Subject: [PATCH 13/19] Don't check prefix until predictions are initialized Makes sure our y offset is correct --- plugins/c9.ide.terminal/predict_echo.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/plugins/c9.ide.terminal/predict_echo.js b/plugins/c9.ide.terminal/predict_echo.js index e1b33f6c..437b560d 100644 --- a/plugins/c9.ide.terminal/predict_echo.js +++ b/plugins/c9.ide.terminal/predict_echo.js @@ -506,7 +506,6 @@ define(function(require, exports, module) { if (lastInput && (state === STATE_WAIT_FOR_PROMPT_OR_ECHO) && lastInput === data.substr(data.length - lastInput.length) - && checkTextBeforePrediction() && (!BASH_ONLY || isBashActive())) { if (DEBUG) console.log(" ^ re-enabled predictions:", lastInput); return startPredict(); @@ -516,7 +515,6 @@ define(function(require, exports, module) { if (lastInput && state == STATE_WAIT_FOR_PROMPT && lastInput === data.substr(data.length - lastInput.length) - && checkTextBeforePrediction() && isBashActive()) { if (DEBUG) console.log(" ^ re-enabled predictions:", lastInput); return startPredict(); From 83c7367d866b9e37204f826579acec034f4aee0d Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Thu, 6 Jul 2017 07:15:30 +0000 Subject: [PATCH 14/19] Fix home command not always working --- plugins/c9.ide.terminal/predict_echo.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/c9.ide.terminal/predict_echo.js b/plugins/c9.ide.terminal/predict_echo.js index 437b560d..2f33ad50 100644 --- a/plugins/c9.ide.terminal/predict_echo.js +++ b/plugins/c9.ide.terminal/predict_echo.js @@ -716,7 +716,7 @@ define(function(require, exports, module) { HomeCommand.tryCreate = function(inputText) { if (INPUTS_HOME.indexOf(inputText) > -1 // Only attempt home if we'd jump to the start of a prompt - && (peek(-predictIndex - 1) === "$" || peek(-predictIndex - 2) === "$")) + && (peek(predictIndex - 1) === "$" || peek(predictIndex - 2) === "$")) return new HomeCommand(inputText); }; function HomeCommand() { From 448cedc3c9baad21efa8798edf9525c7e7994b5e Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Thu, 6 Jul 2017 07:22:35 +0000 Subject: [PATCH 15/19] Minor tweaks --- plugins/c9.ide.terminal/predict_echo.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/c9.ide.terminal/predict_echo.js b/plugins/c9.ide.terminal/predict_echo.js index 2f33ad50..3602cd64 100644 --- a/plugins/c9.ide.terminal/predict_echo.js +++ b/plugins/c9.ide.terminal/predict_echo.js @@ -29,7 +29,7 @@ define(function(require, exports, module) { var MIN_PREDICTION_WAIT = 500; var PING_DEVIATION = 500; - var INSERTABLE_CHARS = /^[A-Za-z0-9!"#$%&'()*+,-\.\/:;<=>?!@[\] ^_`{|}~]+$/; + var INSERTABLE_CHARS = /^[A-Za-z0-9!"#$%&'()*+,-\.\/\\:;<=>?!@[\] ^_`{|}~]+$/; var INPUT_BACKSPACE = "\u007F"; var ESC = "\u001B"; var OUTPUTS_BACKSPACE_ALL = ["\b" + ESC + "[K", "\b" + ESC + "[1K"]; @@ -156,7 +156,7 @@ define(function(require, exports, module) { if (DEBUG) { var alreadyEchoed = predictions[0].before.predict; - console.log("! " + debugPromptSuffix() + console.log("!" + debugPromptSuffix() + predictLine.substr(0, alreadyEchoed.length) + "%c" + predictLine.substr(alreadyEchoed.length), "color: lightblue" @@ -214,7 +214,7 @@ define(function(require, exports, module) { } DEBUG && console.log( - "< " + "<" + (state == STATE_PREDICT ? debugPromptSuffix() + nonPredictTerminal.$debugCharsAt(e.$startY).slice(predictStartX).join("") From 6f26c0da6e37db91d074109c95a4940ef245a4b7 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Thu, 6 Jul 2017 08:40:53 +0000 Subject: [PATCH 16/19] Better handle fringe case --- plugins/c9.ide.terminal/predict_echo.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/c9.ide.terminal/predict_echo.js b/plugins/c9.ide.terminal/predict_echo.js index 3602cd64..8b0d48b6 100644 --- a/plugins/c9.ide.terminal/predict_echo.js +++ b/plugins/c9.ide.terminal/predict_echo.js @@ -324,7 +324,12 @@ define(function(require, exports, module) { var fromChars = target === nonPredictTerminal ? predictChars : nonPredictChars; var toChars = target === nonPredictTerminal ? nonPredictChars : predictChars; + if (!predictChars) { // terminal likely just refreshed, never mind copying to it + state = STATE_WAIT_FOR_PROMPT_OR_ECHO; + return; + } if (!fromChars || !toChars) { + state = STATE_WAIT_FOR_PROMPT_OR_ECHO; errorHandler.reportError(new Error("Warning: can't copy terminal line: "), { fromChars: fromChars, toChars: toChars }); From 107f84cc3a95ccac94972cf3eff745d823f5755c Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Thu, 6 Jul 2017 08:56:11 +0000 Subject: [PATCH 17/19] Stricter failure condition --- plugins/c9.ide.terminal/predict_echo.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/c9.ide.terminal/predict_echo.js b/plugins/c9.ide.terminal/predict_echo.js index c9f9d801..405b3c33 100644 --- a/plugins/c9.ide.terminal/predict_echo.js +++ b/plugins/c9.ide.terminal/predict_echo.js @@ -325,11 +325,11 @@ define(function(require, exports, module) { var toChars = target === nonPredictTerminal ? nonPredictChars : predictChars; if (!predictChars) { // terminal likely just refreshed, never mind copying to it - state = STATE_WAIT_FOR_PROMPT_OR_ECHO; + state = STATE_WAIT_FOR_PROMPT; return; } if (!fromChars || !toChars) { - state = STATE_WAIT_FOR_PROMPT_OR_ECHO; + state = STATE_WAIT_FOR_PROMPT; errorHandler.reportError(new Error("Warning: can't copy terminal line: "), { fromChars: fromChars, toChars: toChars }); From ec83586a08409013f6e9ccfc06bd837a378a08b5 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Mon, 10 Jul 2017 12:08:26 +0000 Subject: [PATCH 18/19] Fix HomeCommand regression --- plugins/c9.ide.terminal/predict_echo.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/c9.ide.terminal/predict_echo.js b/plugins/c9.ide.terminal/predict_echo.js index 405b3c33..3c7ebad8 100644 --- a/plugins/c9.ide.terminal/predict_echo.js +++ b/plugins/c9.ide.terminal/predict_echo.js @@ -721,7 +721,7 @@ define(function(require, exports, module) { HomeCommand.tryCreate = function(inputText) { if (INPUTS_HOME.indexOf(inputText) > -1 // Only attempt home if we'd jump to the start of a prompt - && (peek(predictIndex - 1) === "$" || peek(predictIndex - 2) === "$")) + && (peek(-predictIndex - 1) === "$" || peek(-predictIndex - 2) === "$")) return new HomeCommand(); }; function HomeCommand() { From 4a89c883cb98b3dc1fbef3c2afc0753ac90bc0a7 Mon Sep 17 00:00:00 2001 From: nightwing Date: Sun, 23 Jul 2017 02:15:03 +0400 Subject: [PATCH 19/19] fix predict_echo test --- plugins/c9.ide.terminal/predict_echo_test.js | 31 +++++++------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/plugins/c9.ide.terminal/predict_echo_test.js b/plugins/c9.ide.terminal/predict_echo_test.js index 8b9e2834..9de973d4 100644 --- a/plugins/c9.ide.terminal/predict_echo_test.js +++ b/plugins/c9.ide.terminal/predict_echo_test.js @@ -5,6 +5,7 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/assertions"], function (architect, chai, baseProc) { var expect = chai.expect; + var TMUXNAME = "cloud9test2"; expect.setupArchitectTest([ { @@ -14,7 +15,6 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse debug: true, hosted: true, local: false, - davPrefix: "/" }, "plugins/c9.core/ext", @@ -45,7 +45,7 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse "plugins/c9.ide.ui/forms", { packagePath: "plugins/c9.fs/proc", - tmuxName: "cloud9test2" + tmuxName: TMUXNAME }, "plugins/c9.vfs.client/vfs_client", "plugins/c9.vfs.client/endpoint", @@ -55,16 +55,6 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse baseProc: baseProc }, - // Mock plugins - { - consumes: ["apf", "ui", "Plugin"], - provides: [ - "commands", "menus", "commands", "layout", "watcher", - "save", "anims", "clipboard", "dialog.alert", "auth.bootstrap", - "info", "dialog.error" - ], - setup: expect.html.mocked - }, { consumes: ["tabManager", "proc", "terminal", "terminal.predict_echo", "c9"], provides: [], @@ -105,8 +95,6 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse before(function(done) { this.timeout(45000); - apf.config.setProperty("allow-select", false); - apf.config.setProperty("allow-blur", false); bar.$ext.style.background = "rgba(220, 220, 220, 0.93)"; bar.$ext.style.position = "fixed"; @@ -120,7 +108,7 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse predictor.$setTestTimeouts(); predictor.DEBUG = true; - proc.execFile("~/.c9/bin/tmux", { args: ["-L", "cloud9test", "kill-server"]}, function(err) { + proc.execFile("~/.c9/bin/tmux", { args: ["-L", TMUXNAME, "kill-server"]}, function(err) { tabs.once("ready", function() { tabs.getPanes()[0].focus(); openTerminal(done); @@ -549,15 +537,16 @@ require(["lib/architect/architect", "lib/chai/chai", "/vfs-root", "ace/test/asse }); it("recovers after spurious backspaces on a prompt", function(done) { + var afterBackspace = false; predictor.once("nopredict", function() { - predictor.once("nopredict", function() { - assert.equal(session.$predictor.state, STATE_INITING); - afterPrompt(done); - send("\r"); - }); - send("Q"); + assert.equal(afterBackspace, true); + // assert.equal(session.$predictor.state, STATE_INITING); + afterPrompt(done); + send("\r"); }); send(INPUT_BACKSPACE); + afterBackspace = true; + send("Q"); }); });