Fix #9103, implementing cd - in REPL shell mode

[garrison]

No description provided.

[garrison]

The AppVeyor build failed when attempting to connect to status.julialang.org:443. Is there any way to re-trigger it?

[tkelman]

I restarted it for you. Would be nice to add a test for the new functionality, not sure if/where we have any tests of shell mode right now. Maybe test/repl.jl ?

[garrison]

OK, I made some tests (and I am waiting to see if they work on Windows), but running them displays the new directories to STDOUT it seems.

$ make && make test-repl
    JULIA test/repl
     * repl
/tmp/tmp7qjvvg
/home/garrison/julia/test
/home/garrison
    SUCCESS

Perhaps the call to println() in repl_cmd() should really be writing to a file handle, not STDOUT? This call existed before I made the change in this pull request, though.

[Keno]

Yes, quite likely that call should write to the REPL's stream.

[tkelman]

use @test, not @assert here

[garrison]

At this point it works on UNIX-like systems. The Windows test hangs, however, waiting on the REPL to print back the directory after changing directory. I have no way myself of debugging this further.

[garrison]

To be clear on the status: cd - now works in the shell mode on UNIX, and likely works on Windows. It no longer println's the new directory name to STDOUT, but instead to the output stream of the REPL, which is better for testing. I added tests for the cd shell-mode functionality to test/repl.jl. These tests work on UNIX but hang on Windows. It almost certainly hangs waiting for the directory cd'ed to to be output by the REPL (perhaps a variation on the directory name is instead given? or perhaps it is a CRLF issue, since the current test expects a newline to come immediately after the new directory.).

[tkelman]

@dhoegh would you be willing/able to help debug the windows side of this?

[dhoegh]

It needs to be changed to tmpdir = escape_string(mktempdir()) due to no escaption takes place when you write to stdin_write. This effects windows path due to it uses backslashes, but it also effects linux if there is spaces in the path.

[tkelman]

Looks like this is passing on win32 now, but would there be any way to avoid the WARNING: readuntil(IO,AbstractString) will perform poorly with a long string? That could get a bit annoying, the existing warnings that we get while running the tests have already confused people, would like to avoid adding any more if we can.

[garrison]

@tkelman I fixed the warning by looking for at most 40 characters when calling readuntil().
All tests now pass, and I believe this is ready to be merged.

[tkelman]

Great, looks good to me. If no objections or other feedback I think this should be good to merge.

[tkelman]

Thanks for the contribution @garrison!

[garrison]

Thank you too, @tkelman!