Terminal can't be opened in Windows 10.

[triforcely]

Terminal can't be opened in Windows 10 and method for detecting terminals creates a lot of error message windows.

Expected Behavior

Terminal is opened.

Current Behavior

Following message is displayed for every terminal supported by this library. Finally, target script file is opened in VS Code (I think because it is the default file editor on my PC).

Possible Solution

Detect default terminal executable from system settings instead of using brute-force.
I think that such path is stored in 'ComSpec' env variable in windows.

Context

I tried to attach terminal to my container using "dockly". After investigating the code I noticed that this is caused by this library.

Your Environment

• Library Version used: The one that is shipped with dockly 3.10.1
• Node.js version (e.g. Node.js 5.4): v8.9.4
• Operating System and version (desktop or mobile): Windows 10 Enterprise (1709)

[lirantal]

Thank you so much for sharing this elaborate report Michal.

I assume we could perhaps add the Windows terminal emulators programs to the list like we see in https://github.com/lirantal/opn-shell/blob/master/lib/TerminalLauncher.js#L10 ?

I don't have a Windows machine easily accessible so it would be a bit of a trail&error for me to get this right. Would you be able to clone this lib and try it out with powershell or cmd as fallbacks? It should be relatively easily to test with the bin/cli.js

I'd be happy to merge a PR to add it if you can work it out.

[triforcely]

Adding terminals to the list won't work well because we will still get message box with error for each terminal that is missing. This is working nicely on linux because nothing is displayed in case of missing executable in PATH.

I'll try to come up with something soon 🙂

[lirantal]

Oh, I realized that this is what you meant. Sorry missed that in the original report.

Well, I think it might be an easy one to solve. I can attempt to detect the OS, if the OS is Windows then I use cmd.exe and that's it (not even trying powershell since that will just pop up an error message too). If we wanted to try the shell detection I could also try to locate powershell as an executable/path and only if that exists to launch it.

Actually, that's not the only thing since the script itself is for bash, so even if we had successfully detected cmd.exe it would just fail to run it. So it sounds like we need a .bat file too or whatever Windows does now :-)

I'm here to support any way I can!

[triforcely]

I've been looking into it today and I'm starting to believe that supporting this feature is impossible with 'opn'. When passing 'cmd' or 'powershell' to opn, scripts are executed in the background - no visible terminal is spawned. When explicitly using "start" command, terminal windows are spawned but the child process spawned by nodejs seems to never exit, because underlying shell is not closed - promises are never fulfilled/rejected and nodejs process does not finish.

Working around this line seems not to be worth it, so I'm going to re-work it a little bit. Linux support will work the same way but windows handlers are going to use bare child_process

[lirantal]

It sounds ok to me to fallback to child_process for Windows.
Let's make it happen :)

[triforcely]

Most of the implementation is complete but I still have to write some tests and make linters happy. I'll create PR sometime next week because I won't have time to finish this for the next couple of days.

nvm, after writing that I realized that it would be too lazy.

[lirantal]

Awesome thanks! Will check out the PR soon