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

In shell mode, auto-expand directories so ~ works. #27293

Merged
merged 2 commits into from
May 30, 2018
Merged

Conversation

staticfloat
Copy link
Member

If we're going to allow tab-completion with ~ in shell mode, we should allow cd() to work with it as well. I would argue that all commands should use expanduser() but this PR only does so for ;cd at the moment.

Closes #22181

@staticfloat
Copy link
Member Author

I decided I really like being able to use ~ in other parts of the shell> mode, so this enables it for all shell commands.

@ararslan
Copy link
Member

Possible to add a test?

@staticfloat
Copy link
Member Author

I'm not sure how to test REPL code?

@ararslan
Copy link
Member

There's stuff in stdlib/REPL/test that could be a relevant model for a test.

@staticfloat
Copy link
Member Author

staticfloat commented May 28, 2018

Okay; I added some tests by directly editing stdlib/REPL/test/repl.jl. Hopefully that's the correct thing to do; I'm still not certain how stdlib works in here.

@StefanKarpinski
Copy link
Member

Sure, why not?

@ararslan ararslan added the REPL Julia's REPL (Read Eval Print Loop) label May 29, 2018
@staticfloat
Copy link
Member Author

Sure, why not?

Is that in response to me asking whether editing stdlib/REPL/test/repl.jl is the correct thing to do, or is that an endorsement of the whole PR?

@StefanKarpinski
Copy link
Member

The whole PR.

@staticfloat staticfloat merged commit a29d0d6 into master May 30, 2018
@martinholters martinholters deleted the sf/cd_expand branch May 30, 2018 19:45
@vtjnash
Copy link
Member

vtjnash commented Jun 8, 2018

Please be careful to check that CI can pass before merging (ref #27497).

@vtjnash
Copy link
Member

vtjnash commented Jun 8, 2018

Even if it eventually times out for other reasons on a flaky worker, please at least check that the file with the added test was run.

@StefanKarpinski
Copy link
Member

Is this what was causing the timeouts on Windows?

@staticfloat
Copy link
Member Author

Even if it eventually times out for other reasons on a flaky worker, please at least check that the file with the added test was run.

My bad, I was looking at the REPL precompilation line, not testing line. I checked, just checked the wrong thing. :(

@staticfloat
Copy link
Member Author

Is this what was causing the timeouts on Windows?

Probably; the readuntil() could hang waiting for input that will never come, but what I don't understand is that we have a timer at the top of the script that is supposed to error out after 200 seconds and print a message about the REPL tests not completing properly; our readuntil() should be yielding control to that timer and printing that message.

@vtjnash
Copy link
Member

vtjnash commented Jun 8, 2018

Yes, that Timer is a rather absurd bit of dead code, haha. I've got another commit to replace it with something useful. I plan to open a PR for it once the critical fix PR is merged (see 603359d).

My bad

It happens. I'm pretty sure that CI stability is a self-fulfilling loop, so the real solution is to block merging on any CI failures, and thereby ensure that CI is stable by definition (modulo failures of our CI provider). That's not your fault though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
REPL Julia's REPL (Read Eval Print Loop)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants