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

`workbench.action.terminal.rename` should accept name as an argument #82105

Open
vintprox opened this issue Oct 8, 2019 · 15 comments · May be fixed by #84429

Comments

@vintprox
Copy link

@vintprox vintprox commented Oct 8, 2019

Currently, workbench.action.terminal.rename command doesn't accept any argument. I wanted to make macros that renames current terminal after performing some other actions.

At the moment, it requires user prompt interaction.

public run(entry?: TerminalEntry): Promise<any> {
const terminalInstance = entry ? entry.instance : this.terminalService.getActiveInstance();
if (!terminalInstance) {
return Promise.resolve(undefined);
}
return this.quickInputService.input({
value: terminalInstance.title,
prompt: nls.localize('workbench.action.terminal.rename.prompt', "Enter terminal name"),
}).then(name => {
if (name) {
terminalInstance.setTitle(name, TitleEventSource.Api);
}
});
}

@vscodebot

This comment has been minimized.

Copy link

@vscodebot vscodebot bot commented Oct 8, 2019

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

@Tyriar

This comment has been minimized.

Copy link
Member

@Tyriar Tyriar commented Oct 8, 2019

Have you considered using an extension like https://marketplace.visualstudio.com/items?itemName=fabiospampinato.vscode-terminals for this?

@vintprox

This comment has been minimized.

Copy link
Author

@vintprox vintprox commented Oct 8, 2019

@Tyriar no, I am using custom fork of macros, which is addressed here. I needed some delays between sending terminal sequences, as provided in this example: geddski/macros#9 (comment)

Fortunatelly, it solves issues with providing some dynamic input at right time like passphrase (because I couldn't establish SSH without passphrase prompt, unluckily, so I have to use bad technique atm), running console program on remote computer after SSH login, REPL commands to 3DX MQL.

Thank you for advising fabiospampinato.vscode-terminals extension! I will try it tomorrow on the subject of having right delay between sequences (to replicate macros I've made).

@vintprox

This comment has been minimized.

Copy link
Author

@vintprox vintprox commented Oct 8, 2019

I think I know how to make rename command have simple title argument, but it takes a time to contribute here, hehe.

@perceptron007

This comment has been minimized.

Copy link

@perceptron007 perceptron007 commented Oct 16, 2019

@vintprox @Tyriar I would like to help you guys out, can you explain me a little bit, what changes exactly needs to be done.

@Tyriar

This comment has been minimized.

Copy link
Member

@Tyriar Tyriar commented Oct 16, 2019

@perceptron007 right now the rename command is defined here:

export class RenameTerminalAction extends Action {
public static readonly ID = TERMINAL_COMMAND_ID.RENAME;
public static readonly LABEL = nls.localize('workbench.action.terminal.rename', "Rename");
constructor(
id: string, label: string,
@IQuickOpenService protected quickOpenService: IQuickOpenService,
@IQuickInputService protected quickInputService: IQuickInputService,
@ITerminalService protected terminalService: ITerminalService
) {
super(id, label);
}
public run(entry?: TerminalEntry): Promise<any> {
const terminalInstance = entry ? entry.instance : this.terminalService.getActiveInstance();
if (!terminalInstance) {
return Promise.resolve(undefined);
}
return this.quickInputService.input({
value: terminalInstance.title,
prompt: nls.localize('workbench.action.terminal.rename.prompt', "Enter terminal name"),
}).then(name => {
if (name) {
terminalInstance.setTitle(name, TitleEventSource.Api);
}
});
}
}

I think we need to convert it into a Command to accept an arg like this one:

export class CreateNewWithCwdTerminalCommand extends Command {
public static readonly ID = TERMINAL_COMMAND_ID.NEW_WITH_CWD;
public static readonly LABEL = nls.localize('workbench.action.terminal.newWithCwd', "Create New Integrated Terminal Starting in a Custom Working Directory");
public static readonly CWD_ARG_LABEL = nls.localize('workbench.action.terminal.newWithCwd.cwd', "The directory to start the terminal at");
public runCommand(accessor: ServicesAccessor, args: { cwd: string } | undefined): Promise<void> {
const terminalService = accessor.get(ITerminalService);
const configurationResolverService = accessor.get(IConfigurationResolverService);
const workspaceContextService = accessor.get(IWorkspaceContextService);
const historyService = accessor.get(IHistoryService);
const activeWorkspaceRootUri = historyService.getLastActiveWorkspaceRoot(Schemas.file);
const lastActiveWorkspaceRoot = activeWorkspaceRootUri ? withNullAsUndefined(workspaceContextService.getWorkspaceFolder(activeWorkspaceRootUri)) : undefined;
let cwd: string | undefined;
if (args && args.cwd) {
cwd = configurationResolverService.resolve(lastActiveWorkspaceRoot, args.cwd);
}
const instance = terminalService.createTerminal({ cwd });
if (!instance) {
return Promise.resolve(undefined);
}
terminalService.setActiveInstance(instance);
return terminalService.showPanel(true);
}
}

vintprox added a commit to vintprox/vscode that referenced this issue Oct 18, 2019
@vintprox

This comment has been minimized.

Copy link
Author

@vintprox vintprox commented Oct 18, 2019

@perceptron007 Sorry for late response. I made some changes to code, please review it and cover with some test if needed. I'm not ready to make pull request yet, so I derive it to you, guys.

@Tyriar

This comment has been minimized.

Copy link
Member

@Tyriar Tyriar commented Oct 18, 2019

@vintprox I would have thought you'd need to convert it to a Command, does passing in the arg in a keybinding work there?

We want this keybinding to be able to rename the terminal by executing it:

{
  "key": "cmd+shift+h",
  "command": "workbench.action.terminal.rename",
  "args": {
    "title": "My term"
  }
}

Also you can get intellisense for title, see the workbench.action.terminal.newWithCwd command for how to do this:

image

vintprox added a commit to vintprox/vscode that referenced this issue Oct 18, 2019
microsoft#82105
Feature missing after commit: entry terminal instance probably is not being considered anymore. Requires review.
@vintprox

This comment has been minimized.

Copy link
Author

@vintprox vintprox commented Oct 18, 2019

@Tyriar
Oh, didn't think about that. How do I deal with entry?: TerminalEntry though?
Made another commit for review that excludes possibly ambiguous argument entry.

@Tyriar

This comment has been minimized.

Copy link
Member

@Tyriar Tyriar commented Oct 18, 2019

@vintprox hmm, we may just need to make a new command just like newWithCwd I guess renameActiveTerminal? You're pretty close, you can put a PR out as it will be easier to give feedback there.

@vintprox

This comment has been minimized.

Copy link
Author

@vintprox vintprox commented Oct 18, 2019

You are right! I completely forgot what pull requests are for here.

@sksaifuddin

This comment has been minimized.

Copy link
Contributor

@sksaifuddin sksaifuddin commented Oct 30, 2019

Hello @vintprox did you create the PR ? is this issue resolved ?

@vintprox

This comment has been minimized.

Copy link
Author

@vintprox vintprox commented Oct 30, 2019

Uh-oh, I'm kind of busy right now and cannot find a right time. You can make it, if you want to solve the issue!

@petevdp

This comment has been minimized.

Copy link

@petevdp petevdp commented Nov 10, 2019

Hey, if this issue is open I'd like to pick it up!

@vintprox

This comment has been minimized.

Copy link
Author

@vintprox vintprox commented Nov 10, 2019

Of course, why not, proceed! @petevdp
You can look into my diffs for clues.

@petevdp petevdp linked a pull request that will close this issue Nov 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.