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

REPL: know when to release the prefix context when completing #56322

Conversation

IanButterworth
Copy link
Member

Fixes #55518

@IanButterworth IanButterworth added REPL Julia's REPL (Read Eval Print Loop) completions Tab and autocompletion in the repl labels Oct 25, 2024
@IanButterworth IanButterworth force-pushed the ib/repl_completion_release_prefix_context branch from c762ca9 to 958c1f7 Compare October 25, 2024 03:26

# release context once past a qualified name
for s in ("Base.@time TestInternalBinding", "Base.@time Base.@time TestInternalBinding",
"Base.@time(TestInternalBinding", "@time(Base.@time TestInternalBinding")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be obvious, apologies, but does it also work in e.g Foo.Bar.@macro?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, and yeah
Screenshot 2024-10-25 at 11 06 30 AM
Screenshot 2024-10-25 at 11 12 04 AM
Tests added

@IanButterworth IanButterworth force-pushed the ib/repl_completion_release_prefix_context branch from fcebd2d to 6073643 Compare October 25, 2024 15:17
@IanButterworth
Copy link
Member Author

@aviatesk while this appears to work, one worry I have with this is that this fix should somehow be earlier in the completion handling.

Like, Foo.function(myvar already knows that myvar isn't fixed to the Foo scope before this PR, and that appears to be handled elsewhere.
So maybe detection that we're in macro args space needs to be earlier?

@giordano
Copy link
Contributor

Thanks for looking into this! However examples at #55518 (comment) and #55518 (comment) are still broken:

% julia +pr56322 -q
julia> module Issue55518
       macro name(expr) :($(esc(expr))) end
       const not_really_what_I_wanted = something
       f(; x) = x
       end
Main.Issue55518

julia> using .Issue55518

julia> Issue55518.f(; x=<TAB>

@name
eval
f
include
not_really_what_I_wanted

julia> Issue55518.f; not<TAB>

@IanButterworth
Copy link
Member Author

I feel like REPLCompletions needs to be refactored around JuliaSyntax. There's too much string comparison and regex.

@IanButterworth
Copy link
Member Author

@aviatesk #54858 feels quite breaking, unfortunately. Evidently there just weren't enough tests.

In the context of Mose's examples

julia> Issue55518.f(; x=<TAB>

julia> Issue55518.f; not<TAB>

I don't even know how to approach fixing this, given how REPLCompletions is structured.

I'm going to have to defer to you @aviatesk

@IanButterworth IanButterworth marked this pull request as draft October 28, 2024 20:38
@IanButterworth
Copy link
Member Author

Superseded by #57767

@IanButterworth IanButterworth deleted the ib/repl_completion_release_prefix_context branch March 22, 2025 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completions Tab and autocompletion in the repl REPL Julia's REPL (Read Eval Print Loop)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[REPL] Autocompletion of ModuleName.@macro_name is broken
3 participants