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

Support context module in replcompletions #26930

Merged
merged 4 commits into from Jul 9, 2018

Conversation

5 participants
@jamii
Copy link
Contributor

commented Apr 29, 2018

RE: https://discourse.julialang.org/t/networked-repl/2085/15?u=jamii

@pfitzseb as promised :)

This just adds the context module.

For structured completions, I could:

  • Change the return type of completions and fix the usage in REPL. Then IJulia and co have to change their usage code.
  • Make a new structured_completions function that does all the work and completions calls that and converts the results to strings. Avoids breaking downstream stuff, but leaves a gross api wart.

Which is preferable?

@jamii

This comment has been minimized.

Copy link
Contributor Author

commented Apr 30, 2018

AppVeyor: Build execution time has reached the maximum allowed time for your plan (120 minutes).

Travis: Error in testset SparseArrays/sparsevector:

Doesn't seem to be anything to do with this PR.

@KristofferC

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2018

The 1/10000 hits again? #26909

@pfitzseb

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2018

Looks good to me! I think I'd vastly prefer the first option (changing the return type of completions), since tooling will need to be updated for 0.7 anyways.
What precisely do you want to change the return type to?

Also, are you looking into extending this to enable changing the module the REPL evals into?

@jamii

This comment has been minimized.

Copy link
Contributor Author

commented Apr 30, 2018

What precisely do you want to change the return type to?

A different struct for each completion type, so that clients can render/complete them appropriately. Eg in my emacs autocomplete I need to parse method completions like "string(x::BigInt) in Base.GMP at gmp.jl:568" to know not to insert that literal text into the file. It would be nicer to get MethodCompletion(::Method) and decide what to do with it myself.

Also, are you looking into extending this to enable changing the module the REPL evals into?

Probably not. I don't use the repl much myself and I haven't had much luck integrating my emacs tools with it directly. I would want to send text to an active repl from Julia and have it behave as if it were pasted in, but this seems to be nontrivial. I noticed Juno doesn't show evaled text in the repl either - is this by design or because it's hard?

@pfitzseb

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2018

A different struct for each completion type, so that clients can render/complete them appropriately.

Sounds great.

Probably not. I don't use the repl much myself and I haven't had much luck integrating my emacs tools with it directly.

Alright, fair enough. I'll look into it then :)

I would want to send text to an active repl from Julia and have it behave as if it were pasted in, but this seems to be nontrivial. I noticed Juno doesn't show evaled text in the repl either - is this by design or because it's hard?

It's not trivial, but definitely doable. A super hackish solution would be something like this, but I'm sure one could reuse much more of the REPL implementation. You'd also need to restore what was previously entered on the prompt, but IIRC 0.7 has support for that.

Juno doesn't display evaled text in the REPL by design.

@jamii

This comment has been minimized.

Copy link
Contributor Author

commented Apr 30, 2018

I'm sure one could reuse much more of the REPL implementation

I mostly wanted to handle prompt pasting without implementing it myself, so I would need to push it through the repl parser at least. Probably not the most important thing to focus on right now. I'll look at it again in the future.

@jamii

This comment has been minimized.

Copy link
Contributor Author

commented May 4, 2018

This is more or less there now, except there's a noticeable compilation delay now the first time I complete something in the repl. I probably need to add some precompilation signatures?

@jamii

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2018

Looks like the AppVeyor build timed out.

...except there's a noticeable compilation delay now the first time I complete something in the repl

This disappears when REPL is precompiled via make (I had disabled precompilation for REPL during development).

I think it's good to go @pfitzseb

jamii added some commits Jun 14, 2018

@KristofferC

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2018

Restarting CI since it was a while ago.

@KristofferC KristofferC reopened this Jun 18, 2018

@jamii

This comment has been minimized.

Copy link
Contributor Author

commented Jun 18, 2018

Looks like the first failure is a build timeout, but the second one is interesting:

Error in testset REPL:
Error During Test at /usr/home/julia/julia-fbsd-buildbot/worker/11rel-amd64/build/test/testdefs.jl:19
  Got exception LoadError("/usr/home/julia/julia-fbsd-buildbot/worker/11rel-amd64/build/usr/share/julia/stdlib/v0.7/REPL/test/runtests.jl", 7, LoadError("/usr/home/julia/julia-fbsd-buildbot/worker/11rel-amd64/build/usr/share/julia/stdlib/v0.7/REPL/test/replcompletions.jl", 600, ErrorException("PCRE.exec error: UTF-8 error: isolated byte with 0x80 bit set"))) outside of a @test
  LoadError: LoadError: PCRE.exec error: UTF-8 error: isolated byte with 0x80 bit set
  Stacktrace:
   [1] error at ./error.jl:33 [inlined]
   [2] exec at ./pcre.jl:137 [inlined]
   [3] findnext(::Regex, ::String, ::Int64) at ./regex.jl:222
   [4] #replace#334(::Int64, ::Function, ::String, ::Pair{Regex,String}) at ./strings/util.jl:450
   [5] replace at ./strings/util.jl:444 [inlined]
   [6] #complete_path#11(::Bool, ::Function, ::String, ::Int64) at /usr/home/julia/julia-fbsd-buildbot/worker/11rel-amd64/build/usr/share/julia/stdlib/v0.7/REPL/src/REPLCompletions.jl:263
   [7] #complete_path at ./int.jl:0 [inlined]
   [8] shell_completions(::String, ::Int64) at /usr/home/julia/julia-fbsd-buildbot/worker/11rel-amd64/build/usr/share/julia/stdlib/v0.7/REPL/src/REPLCompletions.jl:734
   [9] test_scomplete(::String) at /usr/home/julia/julia-fbsd-buildbot/worker/11rel-amd64/build/usr/share/julia/stdlib/v0.7/REPL/test/replcompletions.jl:112

While trying to autocomplete /tmp/ in the shell.

@jamii

This comment has been minimized.

Copy link
Contributor Author

commented Jun 18, 2018

I'm not able to replicate this locally. Presumably it depends on what files exist in /tmp. It doesn't seem to be related to this PR though - the error is in a regex that I didn't change.

@KristofferC

This comment has been minimized.

Copy link
Contributor

commented Jun 18, 2018

@jamii

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2018

Looks like #27642 should be fixed. Let's try CI again.

@jamii jamii closed this Jun 21, 2018

@jamii jamii reopened this Jun 21, 2018

@jamii

This comment has been minimized.

Copy link
Contributor Author

commented Jul 5, 2018

One timeout, one OOM. Unrelated.

@KristofferC

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2018

Sorry for this taking time. Since last CI was 14 days ago, we should rerun it.

@KristofferC KristofferC closed this Jul 5, 2018

@KristofferC KristofferC reopened this Jul 5, 2018

@jamii

This comment has been minimized.

Copy link
Contributor Author

commented Jul 5, 2018

No worries. I assume 1.0 is looming :)

@vtjnash

This comment has been minimized.

Copy link
Member

commented Jul 5, 2018

Seems like that doesn't trigger CircleCI

@KristofferC

This comment has been minimized.

Copy link
Contributor

commented Jul 5, 2018

Restarted manually.

@jamii

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2018

We have:

  1. /tmp/julia/bin/julia-debug: No such file or directory
  2. /tmp/julia/bin/julia-debug: No such file or directory
  3. Error in testset FileWatching:
  4. Error in testset download:

The repl tests seem to pass on the platforms where the build gets that far.

@KristofferC KristofferC merged commit 67c19eb into JuliaLang:master Jul 9, 2018

1 of 5 checks passed

ci/circleci: build-i686 Your tests failed on CircleCI
Details
ci/circleci: build-x86_64 Your tests failed on CircleCI
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
julia freebsd ci Build done
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.