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

Properly format file path on when dragging and dropping a tab into the integrated terminal in Windows #30070

Merged
merged 1 commit into from
Jul 20, 2017
Merged
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
5 changes: 5 additions & 0 deletions src/vs/workbench/parts/terminal/common/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -357,4 +357,9 @@ export interface ITerminalInstance {
* Sets the title of the terminal instance.
*/
setTitle(title: string): void;

/**
* Returns the list of nested shells running in the terminal. This is only implemented for Windows.
*/
getShellList(): Promise<string[]>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,47 @@ export class TerminalInstance implements ITerminalInstance {
this._messageTitleListener = null;
}
}

private static getChildProcesses(pid: number): Promise<{executable: string, pid: number}[]> {
return new Promise((resolve, reject) => {
cp.execFile('wmic.exe', ['process', 'where', `parentProcessId=${pid}`, 'get', 'ExecutablePath,ProcessId'], (err, stdout, stderr) => {
if (err) {
reject(err);
} else if (stderr.length > 0) {
resolve([]); // No processes found
} else {
resolve(stdout.split('\n').slice(1).filter(str => str.length > 0).map(str => {
const s = str.split(' ');
return {executable: s[0], pid: Number(s[1])};
}));
}
});
});
}

public async getShellList(): Promise<string[]> {
if (platform.platform !== platform.Platform.Windows) {
return [];
}

const shells = ['bash.exe', 'cmd.exe', 'powershell.exe'];
const pList = [this._shellLaunchConfig.executable];

let pid = this._processId;
while (pid !== null) {
const oldPid = pid;
pid = null;
for (const childproc of await TerminalInstance.getChildProcesses(oldPid)) {
if (shells.indexOf(path.basename(childproc.executable)) !== -1) {
pList.push(childproc.executable);
pid = childproc.pid;
break;
}
}
}

return pList;
}
}

registerThemingParticipant((theme: ITheme, collector: ICssStyleCollector) => {
Expand All @@ -848,4 +889,4 @@ registerThemingParticipant((theme: ITheme, collector: ICssStyleCollector) => {
if (scrollbarSliderActiveBackgroundColor) {
collector.addRule(`.monaco-workbench .panel.integrated-terminal .xterm .xterm-viewport::-webkit-scrollbar-thumb:active { background-color: ${scrollbarSliderActiveBackgroundColor}; }`);
}
});
});
46 changes: 36 additions & 10 deletions src/vs/workbench/parts/terminal/electron-browser/terminalPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,21 +232,47 @@ export class TerminalPanel extends Panel {
return;
}

// Check if the file was dragged from the tree explorer
let uri = e.dataTransfer.getData('URL');
const winFormatters: [RegExp, (uri: URI) => string][] = [
Copy link
Member

Choose a reason for hiding this comment

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

Let's pull this and the application of the formatter into the _preparePathForTerminal function.

// WSL bash
[/^C:\\Windows\\(System32|sysnative)\\bash.exe$/i, uri => '/mnt/' + uri.path[1] + uri.path.substring(3)],
// Git bash
[/bash.exe$/i, uri => uri.path.substring(0, 2) + uri.path.substring(3)],

[/cmd.exe$/i, uri => uri.path[1].toUpperCase() + uri.path.substring(2).replace(/\//g, '\\')],
[/powershell.exe$/i, uri => uri.path[1].toUpperCase() + uri.path.substring(2).replace(/\//g, '\\')],
];

const terminal = this._terminalService.getActiveInstance();
Copy link
Member

Choose a reason for hiding this comment

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

A null check/return on terminal just to be safe.


const uriForm = async (uri: URI) => {
if (platform.isWindows) {
const shells = await terminal.getShellList();
Copy link
Member

Choose a reason for hiding this comment

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

Let's expose a ITerminalInstance.shellExecutable instead of getShellList.

const shell = shells[shells.length - 1];

for (const formatter of winFormatters) {
if (formatter[0].test(shell)) {
return formatter[1](uri);
}
}
}
return uri.path;
};

const uri = e.dataTransfer.getData('URL');
let urip = Promise.resolve(uri);
if (uri) {
uri = URI.parse(uri).path;
urip = uriForm(URI.parse(uri));
} else if (e.dataTransfer.files.length > 0) {
// Check if the file was dragged from the filesystem
uri = URI.file(e.dataTransfer.files[0].path).path;
urip = uriForm(URI.file(e.dataTransfer.files[0].path));
}

if (!uri) {
return;
}

const terminal = this._terminalService.getActiveInstance();
terminal.sendText(this._preparePathForTerminal(uri), false);
urip.then(uri => {
if (!uri) {
return;
}
terminal.sendText(this._preparePathForTerminal(uri), false);
});
}
}));
}
Expand Down