Skip to content

Commit

Permalink
Terminal: Bring back / enable TerminalService API (#20)
Browse files Browse the repository at this point in the history
* Bring back node-pty, update lockfile

* Revert changes to terminalProcess.ts

* Update node to pick up latest 10.x

* Experiment with npmrc

* Revert "Experiment with npmrc"

This reverts commit a0db22b.

* Remove yarnrc

* Remove electron checks

* Remove gc-signals

* it...built?

* Logging / hacks to get terminal service available

* Revert changes to mainThreadHeapService

* Add test case for terminal execution

* Simplify first test to just check for process exit

* Fix bug with windows terminal output

* Add test for  /

* Clean up, add config neeeded to enable terminal

* Get tests green with configuration update

* Remove logging

* Save sandbox test
  • Loading branch information
bryphe authored Jan 17, 2020
1 parent 952a57d commit febdfc8
Show file tree
Hide file tree
Showing 14 changed files with 234 additions and 64 deletions.
3 changes: 0 additions & 3 deletions .yarnrc

This file was deleted.

59 changes: 56 additions & 3 deletions __sandbox__/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,32 @@ let run = async () => {
connection.sendNotification(extMessage, {
type: 4, /* RequestJSONArgs */
reqId: 2,
payload: ["ExtHostConfiguration", "$initializeConfiguration", [{}]],
payload: ["ExtHostConfiguration", "$initializeConfiguration", [{
defaults: {
contents: {
suggest: {
enabled: true
},
terminal: {
integrated: {
env: {
windows: {},
linux: {},
osx: {},
}
}
}
},
keys: ["suggest.enabled", "terminal.integrated"],
overrides: [],
},
user: {},
workspace: {},
folders: {},
isComplete: true,
configurationScopes: {},

}]],
});

connection.sendNotification(extMessage, {
Expand All @@ -96,6 +121,34 @@ let run = async () => {
// }, 1000);


setTimeout(() => {

console.log("SENDING MESSAGE");
connection.sendNotification(extMessage, {
type: 4,
reqId: 3,
payload: ["ExtHostTerminalService", "$createProcess", [
1, {
name: "Terminal 1",
executable: "/non-existent-item",
args: [],
},{
//scheme: "file",
//path: "/Users/bryphe"
},
20,
20
]]
});
connection.sendNotification(extMessage, {
type: 4,
reqId: 3,
payload: ["ExtHostTerminalService", "$acceptProcessInput", [
1, "git status\n"
]]
});
}, 2000);

let testModelAdded = {
uri: {
scheme: "file",
Expand All @@ -116,7 +169,7 @@ let run = async () => {
};


setTimeout(() => {
/*setTimeout(() => {
connection.sendNotification(extMessage, {
type: 4,
reqId: 3,
Expand Down Expand Up @@ -148,7 +201,7 @@ let run = async () => {
true,
]],
});
}, 2000);
}, 2000);*/

let closePromise = new Promise((c) => {
connection.onClose(() => c());
Expand Down
35 changes: 31 additions & 4 deletions __tests__/ExtensionHost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,20 @@ export interface IExtensionHost {
waitForMessageOnce: (rpcName: string, methodName: string, filter?: filterFunc) => Promise<void>;

// ExtHostWorkspace
acceptWorkspaceData(uri: any, name: string, id: string);
acceptWorkspaceData: (uri: any, name: string, id: string) => any;

// ExtHostConfiguration
acceptConfigurationChanged(data: any, configurationChangeData: any);
acceptConfigurationChanged: (data: any, configurationChangeData: any) => any;

createDocument: (uri: any, lines: string[], modeId: string) => void;
updateDocument: (uri: any, range: ChangedEventRange, text: string, version: number) => void;

executeContributedCommand(id: string);
executeContributedCommand: (id: string) => any;

provideCompletionItems: (handle: number, resource: any, position: any, context: any) => Promise<any>;

terminalCreateProcess: (id: number, terminalOptions: any, width: number, height: number) => void;

onMessage: IEvent<any>;
}

Expand Down Expand Up @@ -192,9 +194,18 @@ export let withExtensionHost = async (extensions: string[], f: apiFunction) => {
contents: {
suggest: {
enabled: true
},
terminal: {
integrated: {
env: {
windows: null,
linux: null,
osx: null,
}
}
}
},
keys: ["suggest.enabled"],
keys: ["suggest.enabled", "terminal.integrated"],
overrides: [],
},
user: {},
Expand Down Expand Up @@ -332,6 +343,21 @@ export let withExtensionHost = async (extensions: string[], f: apiFunction) => {
let provideCompletionItems = (handle: number, resource: any, position: any, context: any) => {
return sendRequestWithCancellation(["ExtHostLanguageFeatures", "$provideCompletionItems", [handle, resource, position, context]]);
};
let terminalCreateProcess = (id: number, terminalOptions: any, width: number, height: number) => {
sendNotification([
"ExtHostTerminalService",
"$createProcess",
[
id,
terminalOptions,
{
// TODO: Working directory
},
20,
20,
]
]);
};

let extHost = {
start,
Expand All @@ -344,6 +370,7 @@ export let withExtensionHost = async (extensions: string[], f: apiFunction) => {
executeContributedCommand,
updateDocument,
provideCompletionItems,
terminalCreateProcess,
};

await f(extHost);
Expand Down
80 changes: 80 additions & 0 deletions __tests__/Terminal.Test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import * as path from "path";

import * as ExtensionHost from "./ExtensionHost";

let extensionPath = path.join(__dirname, "..", "test_extensions", "oni-api-tests", "package.json");

describe("Terminal", () => {
test("Verify we get a $sendProcessExit for a command that does not exist", async () => {
await ExtensionHost.withExtensionHost([extensionPath], async (api) => {

let terminalSendProcessExit =
api.waitForMessageOnce("MainThreadTerminalService", "$sendProcessExit", (args) => {
console.log("$sendProcessExit: " + args[1]);
return true;
});

await api.start();

api.terminalCreateProcess(1, {
name: "Terminal 1",
executable: "/non-existent-item",
args: [],
},
20,
20);

await terminalSendProcessExit;
});
});

test("Verify we get a $sendProcessData / $sendProcessTitle for a command that works", async () => {
await ExtensionHost.withExtensionHost([extensionPath], async (api) => {
let terminalSendProcessTitle =
api.waitForMessageOnce("MainThreadTerminalService", "$sendProcessTitle", (args) => {
console.log("$sendProcessTitle: " + args[1]);
return true;
});

let terminalSendProcessData =
api.waitForMessageOnce("MainThreadTerminalService", "$sendProcessData", (args) => {
console.log("$sendProcessData: " + args[1]);
return true;
});

let terminalSendProcessExit =
api.waitForMessageOnce("MainThreadTerminalService", "$sendProcessExit", (args) => {
console.log("$sendProcessExit: " + args[1]);
return true;
});

let terminalArgs: any = null;
if(process.platform == "win32") {
terminalArgs = {
name: "Windows Terminal",
executable: "cmd.exe",
args: ["/c", "echo", "hello"]
}
} else {

terminalArgs = {
name: "Bash Terminal",
executable: "bash",
args: ["-c", "echo hello"]
}
}

await api.start();

api.terminalCreateProcess(
1,
terminalArgs,
20,
20);

await terminalSendProcessTitle;
await terminalSendProcessData;
await terminalSendProcessExit;
});
});
});
5 changes: 3 additions & 2 deletions build/azure-pipelines/common/symbols.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ async function ensureVersionAndSymbols(options: IOptions) {
const pakage = require('../../../package.json');
const product = require('../../../product.json');
const repository = product.electronRepository;
const electronVersion = require('../../lib/electron').getElectronVersion();
// Onivim: We're running via Node, not electron
const electronVersion = "?.?.?";
const insiders = product.quality !== 'stable';
let codeVersion = pakage.version;
if (insiders) {
Expand Down Expand Up @@ -216,4 +217,4 @@ if (repository && codeVersion && electronVersion && (product.quality === 'stable
});
} else {
console.log(`HockeyApp: skipping due to unexpected context (repository: ${repository}, codeVersion: ${codeVersion}, electronVersion: ${electronVersion}, quality: ${product.quality})`);
}
}
2 changes: 1 addition & 1 deletion build/azure-pipelines/continuous-build-oni.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
steps:
- task: NodeTool@0
inputs:
versionSpec: "10.15.1"
versionSpec: "10.x"
- powershell: |
npm install -g [email protected]
displayName: Install jest
Expand Down
3 changes: 2 additions & 1 deletion build/gulpfile.vscode.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ function darwinBundleDocumentType(extensions, icon) {
}

const config = {
version: getElectronVersion(),
//version: getElectronVersion(),
version: "9.9.9",
productAppName: product.nameLong,
companyName: 'Microsoft Corporation',
copyright: 'Copyright (C) 2019 Microsoft. All rights reserved',
Expand Down
16 changes: 9 additions & 7 deletions build/lib/electron.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,23 @@ const path = require('path');
const root = path.dirname(path.dirname(__dirname));

function getElectronVersion() {
const yarnrc = fs.readFileSync(path.join(root, '.yarnrc'), 'utf8');
//const yarnrc = fs.readFileSync(path.join(root, '.yarnrc'), 'utf8');
// @ts-ignore
const target = /^target "(.*)"$/m.exec(yarnrc)[1];
//const target = /^target "(.*)"$/m.exec(yarnrc)[1];

return target;
//return target;
return "9.9.9";
}

module.exports.getElectronVersion = getElectronVersion;

// returns 0 if the right version of electron is in .build/electron
// @ts-ignore
if (require.main === module) {
const version = getElectronVersion();
const versionFile = path.join(root, '.build', 'electron', 'version');
const isUpToDate = fs.existsSync(versionFile) && fs.readFileSync(versionFile, 'utf8') === `v${version}`;
// const version = getElectronVersion();
// const versionFile = path.join(root, '.build', 'electron', 'version');
// const isUpToDate = fs.existsSync(versionFile) && fs.readFileSync(versionFile, 'utf8') === `v${version}`;

process.exit(isUpToDate ? 0 : 1);
// process.exit(isUpToDate ? 0 : 1);
return 0;
}
11 changes: 6 additions & 5 deletions build/npm/preinstall.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@ let err = false;

const majorNodeVersion = parseInt(/^(\d+)\./.exec(process.versions.node)[1]);

// if (majorNodeVersion < 8 || majorNodeVersion >= 11) {
// console.error('\033[1;31m*** Please use node >=8 and <11.\033[0;0m');
// err = true;
// }
// Needed for native dependencies
if (majorNodeVersion < 8 || majorNodeVersion >= 11) {
console.error('\033[1;31m*** Please use node >=8 and <11.\033[0;0m');
err = true;
}

const cp = require('child_process');
const yarnVersion = cp.execSync('yarn -v', { encoding: 'utf8' }).trim();
Expand All @@ -31,4 +32,4 @@ if (!/yarn\.js$|yarnpkg$/.test(process.env['npm_execpath'])) {
if (err) {
console.error('');
process.exit(1);
}
}
3 changes: 2 additions & 1 deletion oni.package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
"graceful-fs": "4.1.11",
"iconv-lite": "0.4.23",
"semver": "^5.5.0",
"vscode-jsonrpc": "4.0.0"
"vscode-jsonrpc": "4.0.0",
"node-pty": "0.8.1"
},
"devDependencies": {
},
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"minimist": "1.2.0",
"native-is-elevated": "^0.2.1",
"native-keymap": "1.2.5",
"native-watchdog": "1.0.0",
"node-pty": "0.8.1",
"semver": "^5.5.0",
"spdlog": "0.7.2",
"sudo-prompt": "8.2.0",
Expand Down
11 changes: 8 additions & 3 deletions src/vs/workbench/api/node/extHostTerminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -464,8 +464,9 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape {
// // Resolve env vars from config and shell
// const lastActiveWorkspaceRoot = this._workspaceContextService.getWorkspaceFolder(lastActiveWorkspaceRootUri);
const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux');
// const envFromConfig = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...terminalConfig.env[platformKey] }, lastActiveWorkspaceRoot);
//const envFromConfig = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...terminalConfig.env[platformKey] }, lastActiveWorkspaceRoot);
const envFromConfig = { ...terminalConfig.env[platformKey] };

// const envFromShell = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...shellLaunchConfig.env }, lastActiveWorkspaceRoot);

// Merge process env with the env from config
Expand All @@ -483,10 +484,14 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape {

// Fork the process and listen for messages
this._logService.debug(`Terminal process launching on ext host`, shellLaunchConfig, initialCwd, cols, rows, env);
const p = new TerminalProcess(shellLaunchConfig, initialCwd, cols, rows, env, terminalConfig.get('windowsEnableConpty'));
// TODO: Bring back `terminalConfig.get('windowsEnableConpty'));`
// When running as a node process, if true, this will push output to stdout and not giving `onProcessData` a chance to intercept
const p = new TerminalProcess(shellLaunchConfig, initialCwd, cols, rows, env, /* windowsEnableConpty */ false);
p.onProcessIdReady(pid => this._proxy.$sendProcessPid(id, pid));
p.onProcessTitleChanged(title => this._proxy.$sendProcessTitle(id, title));
p.onProcessData(data => this._proxy.$sendProcessData(id, data));
p.onProcessData(data => {
this._proxy.$sendProcessData(id, data)
});
p.onProcessExit((exitCode) => this._onProcessExit(id, exitCode));
this._terminalProcesses[id] = p;
}
Expand Down
Loading

0 comments on commit febdfc8

Please sign in to comment.