Fix detached process issue for Windows. #43
Conversation
src/common/AndroidUtils.ts
Outdated
|
|
||
| // NOTE: detaching a process in windows seems to detach the streams. Prevent spawn from detaching when | ||
| // used in Windows OS for special handling of some commands (adb). | ||
| private static spawnChildDetachedIfNeeded( |
There was a problem hiding this comment.
Minor: I don't feel like we really need to describe the "detached" part within the method signature itself. The code is pretty self-explanatory on that front. I'd vote for simply spawnChild().
src/common/AndroidUtils.ts
Outdated
| // -port | ||
| // 5572 | ||
| adjustedPort = this.DEFAULT_ADB_CONSOLE_PORT; | ||
| const portString = '-port'; |
There was a problem hiding this comment.
Minor: can we rename this to portArgumentString? I find in analyzing the code that I'm otherwise expecting this to be a string representation of the port number.
| const portString = '-port'; | ||
| const portStringIndx = data.indexOf(portString); | ||
| if (portStringIndx > -1) { | ||
| const portIndx = data.indexOf( |
There was a problem hiding this comment.
This feels fragile. It feels like we're shoe-horned into searching for the index of a particular numeric string because of the way we're parsing the file. Given that the structure of emu-launch-params.txt is line-by-line, a more intuitive parsing approach to me is to actually parse the file line by line too, rather than by "search" (as it were).
There was a problem hiding this comment.
We can address those when we tackle #8, I suppose.
There was a problem hiding this comment.
I was surprised to find this, but iteration/looping is actually more performant than indexOf. https://medium.com/@mrashes2/indexof-vs-for-loop-6a9f7bd5c646
Disallows detaching of spawned child process on Windows. Fixes parsing of port number from the emu-launch-params.txt. CC: @brandonpage