Terminus doesn't output results of remote commands

[peterromero]

https://github.com/pantheon-systems/terminus/blob/7a337955a512d94480b2227cb1b7b7fbc7cc028d/src/Commands/Remote/SSHBaseCommand.php#L84

Why doesn't this function return $output;? I've been banging my head against the wall for months trying to figure out why I couldn't get a remote drush call to give me a login or a site status... but adding return $output; here fixed that.

[greg-1-anderson]

Terminus does not return output on Windows. Returning $output here might introduce double output on Linux and Mac machines. You are correct though that it is odd that this method does not return anything, but the commands that call it do return. Seems this should be fixed up and commented so that the intentions are clear.

A command should return its output if its output is to be formatted by the output formatters. In the case of these remote commands, the output has already been displayed interactively, so it is not necessary (or desirable) to return the output for formatting.

As of yet, no one has tracked down the reason why interactive output is lost on Windows machines.

[peterromero]

I'd recommend then, that we remove the @return string from the docblock, and more importantly, make it prominent on https://pantheon.io/docs/terminus/ that Terminus no longer works with Windows (the tech support people at Pantheon should also be notified, as they don't currently know this). On a side note, where would be a good place to start searching for the bug that prevents Windows from displaying interactive output? I'm not familiar with how this works.

[dustinleblanc]

We think this might be causing some issues in our automated test suite for the Pantheon Lando recipe. We're refactoring the lando pull command to use terminus remote:drush and terminus remote:wp for database dumping (because they are way faster than directly hitting mysql) and our Circle CI tests are hanging indefinitely (seemingly waiting for an exit from Terminus). @greg-1-anderson I think you mentioned that the lack of any return here is odd, and I am curious if explicitly returning a string from this would improve the situation.

We've now built a similar integration for Platform.sh and looking at the platform CLI, dumping the database does result in executing a remote SSH command and then returning it's result: https://github.com/platformsh/platformsh-cli/blob/master/src/Command/Db/DbDumpCommand.php#L265

There are obviously things I don't know here, but I do wonder if we'd be okay returning the remote exit code