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

Allow Terminal URI opener to accept a JSON-stringified config argument #77

Closed
wants to merge 1 commit into from

Conversation

edjubuh
Copy link
Contributor

@edjubuh edjubuh commented Mar 30, 2018

This allows other plugins to create terminal tabs by doing something like:

atom.workspace.open(`terminal-tab:///${JSON.stringify({shellPath: pathToCustomShell})`);

This is a particularly interesting feature for Windows, where users sometimes use several different shells - Command Prompt, PowerShell, WSL, or Git Bash. Down the road, we could have a prompt command to create a terminal with a custom shell, as opposed to needing to manually reconfigure the default every time.

@edjubuh edjubuh changed the title Allow Terminal URI opener accept a JSON-stringified config argument Allow Terminal URI opener to accept a JSON-stringified config argument Mar 30, 2018
@edjubuh
Copy link
Contributor Author

edjubuh commented Mar 30, 2018

Not sure why tests are failing - they seem to be failing on master as well.

An alternative design to this is to provide a service to other packages. This would be a bit safer since there's no JSON.stringify/JSON.parse-ing happening. I'd prefer the service-based solution, but the URI-based solution a quick way to get service-like functionality without needing to add any new methods.

With the service, we modify package.json and export the following method:

provideTerminalSession() {
  return (config={}) => atom.workspace.open(new TerminalSession(config));
}

a consumer package would then do something like this:

createTerminalTab = null;
consumeTerminalTab(_createTerminalTab) {
    createTerminalTab = _createTerminalTab; 
    return new Disposable(() => { createTerminalTab = null; });
}
// somewhere else, probably in activate()
atom.commands.add('my-package:open-terminal', () => createTerminalTab({shellPath: pathToCustomShell}));

@edjubuh
Copy link
Contributor Author

edjubuh commented Apr 18, 2018

Any update here?

The Atom service diff can be found here: master...edjubuh:service

@jsmecham
Copy link
Owner

jsmecham commented Apr 21, 2018

Providing a service is certainly the preferred option here. However, I want to proceed very cautiously due to the obvious security concerns.

I am going to spend a little time this weekend playing around with services and the various use cases as they pertain to opening shells and running commands.

@edjubuh
Copy link
Contributor Author

edjubuh commented Apr 21, 2018

Closing in favor of #85

@edjubuh edjubuh closed this Apr 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants