Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Versioning].
- solve the problem of failed parsing of containers ([@henryriley0])
- Fixes #421 - Added `registerLimit` option to specify the registers to
display - PR #444 ([@chenzhiy2001])
- add integrated terminal support ([@henryriley0])
Copy link
Collaborator

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


## [0.27.0] - 2024-02-07

Expand Down
2 changes: 1 addition & 1 deletion src/backend/linux/console.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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\""]);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we still provide the option to use a preferedEmulator?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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)) {
Expand Down
8 changes: 5 additions & 3 deletions src/backend/mi2/mi2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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?!?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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);
}
Expand Down
16 changes: 16 additions & 0 deletions src/gdb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export interface AttachRequestArguments extends DebugProtocol.AttachRequestArgum
}

class GDBDebugSession extends MI2DebugSession {
protected supportsRunInTerminalRequest = false;
protected override initializeRequest(response: DebugProtocol.InitializeResponse, args: DebugProtocol.InitializeRequestArguments): void {
response.body.supportsGotoTargetsRequest = true;
response.body.supportsHitConditionalBreakpoints = true;
Expand All @@ -54,6 +55,7 @@ class GDBDebugSession extends MI2DebugSession {
response.body.supportsSetVariable = true;
response.body.supportsStepBack = true;
response.body.supportsLogPoints = true;
args.supportsRunInTerminalRequest = true;
this.sendResponse(response);
}

Expand Down Expand Up @@ -98,6 +100,20 @@ class GDBDebugSession extends MI2DebugSession {
});
} else {
this.miDebugger.load(args.cwd, args.target, args.arguments, args.terminal, args.autorun || []).then(() => {
if(this.miDebugger.application.includes('gdb')){
if(args.terminal === 'integrated' || args.terminal === '' || args.terminal === undefined){
const terminalRequestArgs:DebugProtocol.RunInTerminalRequestArguments = {
kind: "integrated",
title: this.miDebugger.application,
cwd: args.cwd || '',
args: [],
};
if(process.platform != "win32"){
this.createIntegratedTerminalLinux(terminalRequestArgs);
this.emit("debug-ready");
}
}
}
this.sendResponse(response);
}, err => {
this.sendErrorResponse(response, 103, `Failed to load MI Debugger: ${err.toString()}`);
Expand Down
73 changes: 73 additions & 0 deletions src/mibase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)}`;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using the same approach as above for the tempName?
Math.floor(Math.random() * 100000000).toString(36)

Copy link
Contributor Author

@HenryRiley0 HenryRiley0 Dec 4, 2024

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just fo0r understanding: Is the first command slow and the mv is used to ensure it is finished before it is available under the name?

Copy link
Contributor Author

@HenryRiley0 HenryRiley0 Dec 5, 2024

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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) {
Expand Down
Loading