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

?(x, y)TAB completes methods accepting x, y #38791

Merged
merged 5 commits into from
Aug 9, 2021
Merged

Conversation

timholy
Copy link
Sponsor Member

@timholy timholy commented Dec 9, 2020

A number of calls to change the language have been justified in part by arguments for better tab completion. It seems time to separate these issues by improving discoverability in a way that integrates with multiple dispatch. With this PR, ?(x, y)TAB completes methods that can be called with (x, y), and ?(x, yTAB (without the closing )) completes methods called with (x, y, ...). There is also limited module-scoping available (only one layer deep).

An alternative to ? would be to have the cursor positioned where the function name should be supplied.

I'm happy to discuss changes to this, but depending on whether a couple of other deadlines evaporate I might not have time to make any substantive changes here for a while...so let me say at the outset that I'd be just fine with someone taking this over if desired. Or, it can sit until I get time.

Demos:

julia> InteractiveUtils.?("hello")[TAB]
apropos(string) in REPL at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/REPL/src/docview.jl:727
clipboard(x) in InteractiveUtils at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/InteractiveUtils/src/clipboard.jl:60
code_llvm(f) in InteractiveUtils at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/InteractiveUtils/src/codeview.jl:178
code_native(f) in InteractiveUtils at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/InteractiveUtils/src/codeview.jl:199
edit(path::AbstractString) in InteractiveUtils at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/InteractiveUtils/src/editless.jl:195
edit(f) in InteractiveUtils at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/InteractiveUtils/src/editless.jl:223
eval(x) in InteractiveUtils at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/InteractiveUtils/src/InteractiveUtils.jl:3
include(x) in InteractiveUtils at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/InteractiveUtils/src/InteractiveUtils.jl:3
less(file::AbstractString) in InteractiveUtils at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/InteractiveUtils/src/editless.jl:256
less(f) in InteractiveUtils at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/InteractiveUtils/src/editless.jl:264
report_bug(kind) in InteractiveUtils at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/InteractiveUtils/src/InteractiveUtils.jl:385
separate_kwargs(args...; kwargs...) in InteractiveUtils at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/InteractiveUtils/src/macros.jl:7

julia> InteractiveUtils.?("hello")[SHIFT-TAB]   # this excludes duck-typed methods
edit(path::AbstractString) in InteractiveUtils at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/InteractiveUtils/src/editless.jl:195
less(file::AbstractString) in InteractiveUtils at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/InteractiveUtils/src/editless.jl:256

julia>  ?([1 2; 3 4], 1, 1)[SHIFT-TAB]
LinRange(start, stop, len::Integer) in Base at range.jl:427
StepRangeLen(ref::R, step::S, len::Integer) where {R, S} in Base at range.jl:366
broadcast(f, x::Number...) in Base.Broadcast at broadcast.jl:824
broadcast(f, avs::Union{Number, LinearAlgebra.Adjoint{T, var"#s828"} where var"#s828"<:(AbstractVector{T} where T) where T}...) in LinearAlgebra at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/LinearAlgebra/src/adjtrans.jl:281
broadcast(f, tvs::Union{Number, LinearAlgebra.Transpose{T, var"#s828"} where var"#s828"<:(AbstractVector{T} where T) where T}...) in LinearAlgebra at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/LinearAlgebra/src/adjtrans.jl:282
broadcast(f::Tf, As...) where Tf in Base.Broadcast at broadcast.jl:821
broadcast!(f::Tf, dest, As::Vararg{Any, N}) where {Tf, N} in Base.Broadcast at broadcast.jl:860
checkbounds(A::AbstractArray, I...) in Base at abstractarray.jl:608
circshift!(dest::AbstractArray, src, shiftamt) in Base at multidimensional.jl:1116
clamp(x::X, lo::L, hi::H) where {X, L, H} in Base.Math at math.jl:65
clamp!(x::AbstractArray, lo, hi) in Base.Math at math.jl:93
copyto!(dest::AbstractArray, dstart::Integer, src) in Base at abstractarray.jl:846
error(s::Vararg{Any, N}) where N in Base at error.jl:40
fill(v, dims::Union{Integer, AbstractUnitRange}...) in Base at array.jl:449
get(A::AbstractArray, i::Integer, default) in Base at abstractarray.jl:1411
getindex(A::Array, i1::Int64, i2::Int64, I::Int64...) in Base at array.jl:788
getindex(A::Array, i1::Integer, I::Integer...) in Base at abstractarray.jl:1167
getindex(A::Array, i1::Union{Integer, CartesianIndex}, I::Union{Integer, CartesianIndex}...) in Base at multidimensional.jl:637
getindex(A::AbstractArray, I...) in Base at abstractarray.jl:1161
isassigned(a::Array, i::Int64...) in Base at array.jl:201
isassigned(a::AbstractArray, i::Integer...) in Base at abstractarray.jl:505
map(f, x::Number, ys::Number...) in Base at number.jl:238
mapreduce(f, op, a::Number) in Base at reduce.jl:419
rand(X, d::Integer, dims::Integer...) in Random at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.7/Random/src/Random.jl:283
repeat(A::AbstractArray, counts...) in Base at abstractarraymath.jl:223
reshape(parent::AbstractArray, dims::Int64...) in Base at reshapedarray.jl:116
reshape(parent::AbstractArray, dims::Union{Int64, AbstractUnitRange}...) in Base at reshapedarray.jl:110
reshape(parent::AbstractArray, dims::Union{Colon, Int64}...) in Base at reshapedarray.jl:117
selectdim(A::AbstractArray, d::Integer, i) in Base at abstractarraymath.jl:122
setindex!(A::Array{T, N} where N, x, i1::Int64) where T in Base at array.jl:825
setindex!(A::Array, v, i1::Union{Integer, CartesianIndex}, I::Union{Integer, CartesianIndex}...) in Base at multidimensional.jl:639
setindex!(A::AbstractArray, v, I...) in Base at abstractarray.jl:1258
similar(a::AbstractArray{T, N} where N, dims::Union{Integer, AbstractUnitRange}...) where T in Base at abstractarray.jl:735
view(A::AbstractArray, I::Vararg{Any, N}) where N in Base at subarray.jl:164

Closes #30052
xref #38704
xref #37993

@timholy timholy added the stdlib:REPL Julia's REPL (Read Eval Print Loop) label Dec 9, 2020
@johnnychen94
Copy link
Sponsor Member

Any chance we formally make an API for the matched methods? Ask this because it allows other REPL tools to work smarter based on the results. For example, thefix.jl

@rapus95
Copy link
Contributor

rapus95 commented Dec 9, 2020

does it work with keyword arguments?

EDIT: that following underscore usage doesn't make sense
How easy would it be to allow _ as placeholder for arguments that I don't care over? I.e. say I forgot the name of a certain function
julia>((x,y)->(x,y), _, init=5)TAB which in best case would show me reduce/foldr/foldl

@timholy
Copy link
Sponsor Member Author

timholy commented Dec 9, 2020

Any chance we formally make an API for the matched methods? Ask this because it allows other REPL tools to work smarter based on the results. For example, thefix.jl

I'm not sure how TheFix works internally, but the matches come back as a list of MethodCompletions:

struct MethodCompletion <: Completion
func
input_types::Type
method::Method
orig_method::Union{Nothing,Method} # if `method` is a keyword method, keep the original method for sensible printing
end

Does that address the concern?

@johnnychen94
Copy link
Sponsor Member

johnnychen94 commented Dec 9, 2020

Great, so basically we can call complete_any_methods and doing some additional REPL work (not just print it out) based on its results, am I understand it right? If so, we could make it an advanced version of methodswith.

julia> methodswith(String, InteractiveUtils; supertypes=true)
[1] edit(path::AbstractString) in InteractiveUtils at /Applications/Julia-1.5.app/Contents/Resources/julia/share/julia/stdlib/v1.5/InteractiveUtils/src/editless.jl:193
[2] edit(path::AbstractString, line::Integer) in InteractiveUtils at /Applications/Julia-1.5.app/Contents/Resources/julia/share/julia/stdlib/v1.5/InteractiveUtils/src/editless.jl:193
[3] less(file::AbstractString) in InteractiveUtils at /Applications/Julia-1.5.app/Contents/Resources/julia/share/julia/stdlib/v1.5/InteractiveUtils/src/editless.jl:255
[4] less(file::AbstractString, line::Integer) in InteractiveUtils at /Applications/Julia-1.5.app/Contents/Resources/julia/share/julia/stdlib/v1.5/InteractiveUtils/src/editless.jl:242

@timholy
Copy link
Sponsor Member Author

timholy commented Dec 9, 2020

If so, we could make it an advanced version of methodswith.

Along those lines, see #33883. There didn't seem to be much interest, and after I developed MethodAnalysis I decided I was probably subverting the real purpose of methodswith.

@codecov-io
Copy link

codecov-io commented Dec 9, 2020

Codecov Report

Merging #38791 (a1106b8) into master (ffc340c) will increase coverage by 6.00%.
The diff coverage is n/a.

❗ Current head a1106b8 differs from pull request most recent head c3c9540. Consider uploading reports for the commit c3c9540 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #38791      +/-   ##
==========================================
+ Coverage   81.55%   87.55%   +6.00%     
==========================================
  Files         350      388      +38     
  Lines       73999    74498     +499     
==========================================
+ Hits        60351    65228    +4877     
+ Misses      13648     9270    -4378     
Impacted Files Coverage Δ
base/download.jl 0.00% <0.00%> (-80.00%) ⬇️
stdlib/REPL/src/TerminalMenus/util.jl 0.00% <0.00%> (-51.29%) ⬇️
base/options.jl 45.83% <0.00%> (-50.01%) ⬇️
base/ttyhascolor.jl 0.00% <0.00%> (-46.67%) ⬇️
stdlib/SharedArrays/src/SharedArrays.jl 38.92% <0.00%> (-43.25%) ⬇️
base/hashing.jl 52.94% <0.00%> (-33.34%) ⬇️
base/c.jl 55.17% <0.00%> (-31.67%) ⬇️
base/deprecated.jl 37.33% <0.00%> (-27.31%) ⬇️
stdlib/REPL/src/TerminalMenus/AbstractMenu.jl 42.48% <0.00%> (-26.80%) ⬇️
base/idset.jl 66.66% <0.00%> (-23.81%) ⬇️
... and 328 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ffc340c...c3c9540. Read the comment docs.

@miguelraz
Copy link
Contributor

I may be speaking out of turn here, but does this PR also hook into the pretty-printing stack-traces mechanism? Having a uniform display style for Tab completions, methodswith, (x, y)TAB and stacktraces would be great.

@KristofferC KristofferC mentioned this pull request Mar 4, 2021
14 tasks
stdlib/REPL/src/REPL.jl Outdated Show resolved Hide resolved
@vtjnash
Copy link
Sponsor Member

vtjnash commented Apr 16, 2021

bump? I think this is nearly complete!

@vtjnash vtjnash added the status:forget me not PRs that one wants to make sure aren't forgotten label May 4, 2021
@oscardssmith
Copy link
Member

Can we get this into 1.7?

@timholy
Copy link
Sponsor Member Author

timholy commented Jul 25, 2021

This should be working again. I've incorporated the feedback above.

We might want to play with this a bit before merging; in 2-argument methods, I keep wanting a way to say that neither argument should be typed as ::Any, but it might be bad if we hard-wired that in. Likewise, we don't have a way of saying "this argument could go in any position."

@rapus95
Copy link
Contributor

rapus95 commented Jul 26, 2021

This should be working again. I've incorporated the feedback above.

We might want to play with this a bit before merging; in 2-argument methods, I keep wanting a way to say that neither argument should be typed as ::Any, but it might be bad if we hard-wired that in. Likewise, we don't have a way of saying "this argument could go in any position."

How about introducing post- or pre-parenthesis modifiers/flags? we could even think about having both and making pre-fix for explicit inclusion and post-fix for explicit exclusion and omitting for some default. (or vice versa)
?a(4,2) a for all/any included

but given that it wouldn't be generic enough, we could either create new struct types which can encode these properties/flags or we could establish another syntax. Like for example a string macro since these support flags by default? Kinda like the var"name" macro.

new struct types: ?(Strict(objA), Arbitrary(objB), objC)
kinda like introducing new traits! (where Openness(o::Any) = AnythingButAny(o))

string macro: ?(arg"objA"s, arg"objB"a, objC)

@timholy
Copy link
Sponsor Member Author

timholy commented Aug 4, 2021

I've been using this for a while now and it seems reasonable even if not as flexible as one might ideally want. While I see the good things about @rapus95's suggestions, I think it would add a lot of visual noise and complexity. If we really want to have fine-grained control, I think the right approach would be to introduce a TerminalMenus.MultiSelectMenu to control this interactively. But perhaps we should first merge this and see what people think of the simple version. We have the whole 1.8 cycle to change our mind.

@rapus95
Copy link
Contributor

rapus95 commented Aug 4, 2021

I definitely support @timholy's suggestion on going with the simple version first and just want to contribute another idea for the interactive refinement suggested (because MultiSelectMenu eats a lot of space, especially if we want it for each argument slot):
How about entering an interactive mode after hitting “ENTER” which then can be controlled via flag keys? I.e.
julia>?(5, "text")
--> Interactive exploration
--> load all matching methods
*have a cursor in the argument places. Then you can use the arrows to select a slot and certain flag keys to toggle the flag on a per-argument basis.
*hitting 'a' will toggle the “all/any” option

Such an interactive approach could also allow for a) supplying substrings that should be part of the function name and b) supplying substrings that should be part of an argument name. Implementation wise, these would just be filters on potentially HUGE lists.

P.S. would having instant toggle keys for some elements in the MultiSelectMenu be a good idea in general? I.e., underline a key in each option, which then can be used to toggle that option w/o moving the cursor there.

@carstenbauer
Copy link
Member

Sorry if this has been mentioned before (I didn't read everything), but can we / do we want to support partial autocompletion, i.e. isapp?(x, y) which, for example, would suggest isapprox(x,y)?

@timholy
Copy link
Sponsor Member Author

timholy commented Aug 4, 2021

can we / do we want to support partial autocompletion, i.e. isapp?(x, y) which, for example, would suggest isapprox(x,y)?

Certainly feasible, but don't we essentially have that already?

julia> apropos("isapp")
Base.@static
Base.isapprox
Base.:
Base.SimdLoop.@simd
Base.Sys.isbsd
Base.Sys.isapple

This is more about searching-by-types than searching-by-name. I guess one could combine them, but I'm not sure if that really would add enough value to compensate.

@timholy timholy merged commit e87e30c into master Aug 9, 2021
@timholy timholy deleted the teh/tab_methsearch branch August 9, 2021 11:16
aviatesk added a commit to JunoLab/FuzzyCompletions.jl that referenced this pull request Aug 9, 2021
aviatesk added a commit to JunoLab/Atom.jl that referenced this pull request Aug 9, 2021
aviatesk added a commit to JunoLab/FuzzyCompletions.jl that referenced this pull request Aug 9, 2021
aviatesk added a commit to JunoLab/Atom.jl that referenced this pull request Aug 9, 2021
@dpinol
Copy link

dpinol commented Nov 2, 2021

I don't see this in 1.7rc2
Do we have to wait until 1.8?

@timholy
Copy link
Sponsor Member Author

timholy commented Nov 2, 2021

Yes.

@dpinol
Copy link

dpinol commented Feb 24, 2022

How's the UX?
If I copy-paste the "?" character, it works fine.
But if I press "?", it enters into help mode and I can't get the autocomplete to work.

@timholy
Copy link
Sponsor Member Author

timholy commented Feb 24, 2022

From the julia> prompt, type exactly these characters:

??(3, "hello")TAB

@dpinol
Copy link

dpinol commented Feb 24, 2022

@timholy thanks!

@DilumAluthge DilumAluthge removed the status:forget me not PRs that one wants to make sure aren't forgotten label Sep 6, 2022
aviatesk added a commit that referenced this pull request Dec 20, 2023
Fix #52551.

This PR ensures that a `SomeModule.?(...#TAB` completion can only
suggests method `foo` such that `SomeModule.foo` exists (by checking
`isdefined(SomeModule, :foo)`). This is equivalent, I believe, to the
initial implementation of #38791,
less the bug.

Now that we have #51345, we may want to relax the above condition
somewhat to include public names present in modules loaded into
`SomeModule`, so that, for instance, a direct completion of `?(` would
include `@assume_effects`. This could be good for method exploration
because even though typing `@assume_effects` with no qualification in
`Main` will error, the error now includes the helpful message
```
Hint: a global variable of this name also exists in Base.
```
But that can wait for a later PR anyway, this one is just the bugfix.

The bug mentioned at
#52551 (comment)
dates from the initial #38791 so this could be backported as far back as
v1.8.

---------

Co-authored-by: Shuhei Kadowaki <[email protected]>
KristofferC pushed a commit that referenced this pull request Dec 23, 2023
Fix #52551.

This PR ensures that a `SomeModule.?(...#TAB` completion can only
suggests method `foo` such that `SomeModule.foo` exists (by checking
`isdefined(SomeModule, :foo)`). This is equivalent, I believe, to the
initial implementation of #38791,
less the bug.

Now that we have #51345, we may want to relax the above condition
somewhat to include public names present in modules loaded into
`SomeModule`, so that, for instance, a direct completion of `?(` would
include `@assume_effects`. This could be good for method exploration
because even though typing `@assume_effects` with no qualification in
`Main` will error, the error now includes the helpful message
```
Hint: a global variable of this name also exists in Base.
```
But that can wait for a later PR anyway, this one is just the bugfix.

The bug mentioned at
#52551 (comment)
dates from the initial #38791 so this could be backported as far back as
v1.8.

---------

Co-authored-by: Shuhei Kadowaki <[email protected]>
(cherry picked from commit a987f56)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib:REPL Julia's REPL (Read Eval Print Loop)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

list of available methods in REPL