-
Notifications
You must be signed in to change notification settings - Fork 118
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add integrated terminal support #447
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ import * as fs from "fs"; | |
export function spawnTerminalEmulator(preferedEmulator: string): Thenable<string> { | ||
return new Promise((resolve, reject) => { | ||
const ttyFileOutput = "/tmp/vscode-gdb-tty-0" + Math.floor(Math.random() * 100000000).toString(36); | ||
ChildProcess.spawn(preferedEmulator || "x-terminal-emulator", ["-e", "sh -c \"tty > " + ttyFileOutput + " && sleep 4294967294\""]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we still provide the option to use a preferedEmulator? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok |
||
ChildProcess.spawn("x-terminal-emulator", ["-e", "sh -c \"tty > " + ttyFileOutput + " && sleep 4294967294\""]); | ||
let it = 0; | ||
const interval = setInterval(() => { | ||
if (fs.existsSync(ttyFileOutput)) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ import * as linuxTerm from '../linux/console'; | |
import * as net from "net"; | ||
import * as fs from "fs"; | ||
import * as path from "path"; | ||
import { DebugProtocol } from 'vscode-debugprotocol'; | ||
import { Client, ClientChannel, ExecOptions } from "ssh2"; | ||
|
||
export function escape(str: string) { | ||
|
@@ -116,19 +117,20 @@ export class MI2 extends EventEmitter implements IBackend { | |
resolve(undefined); | ||
}, reject); | ||
} else { | ||
if (separateConsole !== undefined) { | ||
if (separateConsole == "external") { | ||
linuxTerm.spawnTerminalEmulator(separateConsole).then(tty => { | ||
promises.push(this.sendCommand("inferior-tty-set " + tty)); | ||
promises.push(...autorun.map(value => { return this.sendUserInput(value); })); | ||
Promise.all(promises).then(() => { | ||
this.emit("debug-ready"); | ||
if(this.application.includes('gdb')){ | ||
this.emit("debug-ready"); | ||
} | ||
Comment on lines
+125
to
+127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So no debug-ready for non GDB scenarios and none for win-32 any more?!? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For now, that's the situation. In the future, we can try to add more, but it might involve more work. |
||
resolve(undefined); | ||
}, reject); | ||
}); | ||
} else { | ||
promises.push(...autorun.map(value => { return this.sendUserInput(value); })); | ||
Promise.all(promises).then(() => { | ||
this.emit("debug-ready"); | ||
resolve(undefined); | ||
}, reject); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -776,7 +776,80 @@ export class MI2DebugSession extends DebugSession { | |
this.sourceFileMap = new SourceFileMap(configMap, fallbackGDB); | ||
} | ||
} | ||
public async createIntegratedTerminalLinux(args:DebugProtocol.RunInTerminalRequestArguments) { | ||
const mkdirAsync = fs.promises.mkdir; | ||
const mkdtempAsync = async (tempDir: string, prefix: string): Promise<string> => { | ||
const name = `${prefix}-${Date.now()}-${Math.floor(Math.random() * 1e9)}`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about using the same approach as above for the tempName? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks, i will fix this |
||
const newDirPath = `${tempDir}/${name}`; | ||
|
||
try { | ||
await mkdirAsync(newDirPath, { recursive: true }); | ||
return newDirPath; | ||
} catch (err) { | ||
throw new Error(`Error creating temp directory: ${err.message}`); | ||
} | ||
}; | ||
const ttyTmpDir = await mkdtempAsync(os.tmpdir(), 'debug'); | ||
|
||
(async () => { | ||
try { | ||
fs.writeFileSync( | ||
`${ttyTmpDir}/get-tty`, | ||
`#!/usr/bin/env sh | ||
# reset terminal to clear any previous states | ||
reset | ||
Comment on lines
+798
to
+800
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This uses the shell's init commands, no? Should this be suppressed? Should there be an option to add extra commands? |
||
echo "The input and output of program will be here." | ||
echo "Warning of set controlling terminal fail can be ignored." | ||
tty > ${ttyTmpDir}/ttynameTmp | ||
mv ${ttyTmpDir}/ttynameTmp ${ttyTmpDir}/ttyname | ||
Comment on lines
+803
to
+804
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just fo0r understanding: Is the first command slow and the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The use of mv in this context is not primarily to address slowness but to ensure atomicity and consistency, preventing the watcher from observing a file that is being written to halfway through the process. |
||
# wait for debug to finish | ||
# prefer using tail to detect PID exit, but that requires GNU tail | ||
tail -f --pid=${process.pid} /dev/null 2>/dev/null || while kill -s 0 ${process.pid} 2>/dev/null; do sleep 1s; done | ||
Comment on lines
+806
to
+807
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this mean you consider the PR unfinished / a working draft? |
||
# cleanup | ||
rm ${ttyTmpDir}/ttyname | ||
rm ${ttyTmpDir}/get-tty | ||
rmdir ${ttyTmpDir} | ||
` | ||
); | ||
|
||
let watcher: fs.FSWatcher | undefined; | ||
const ttyNamePromise = new Promise<string>((resolve) => { | ||
watcher = fs.watch(ttyTmpDir, (_eventType, filename) => { | ||
if (filename === 'ttyname') { | ||
watcher?.close(); | ||
resolve( | ||
fs.readFileSync(`${ttyTmpDir}/ttyname`).toString().trim() | ||
); | ||
} | ||
}); | ||
}); | ||
|
||
args.args = ['/bin/sh', `${ttyTmpDir}/get-tty`]; | ||
const response = await new Promise<DebugProtocol.Response>( | ||
(resolve) => | ||
this.sendRequest( | ||
'runInTerminal', | ||
args, | ||
10000, | ||
resolve | ||
) | ||
); | ||
if (response.success) { | ||
const tty = await ttyNamePromise; | ||
await this.miDebugger.sendCommand(`inferior-tty-set ${tty}`).then(done => { | ||
this.miDebugger.emit("debug-ready"); | ||
}); | ||
return; | ||
} else { | ||
watcher?.close(); | ||
const message = `could not start the terminal on the client: ${response.message}`; | ||
throw new Error(message); | ||
} | ||
} catch (err) { | ||
throw new Error(`Error createIntegratedTerminalLinux: ${err.message}`); | ||
} | ||
})(); | ||
} | ||
} | ||
|
||
function prettyStringArray(strings: any) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please reference the issue #75 and/or PR #447 here as well