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

make using .A syntax work #2955

Merged
merged 1 commit into from
Sep 9, 2024
Merged

make using .A syntax work #2955

merged 1 commit into from
Sep 9, 2024

Conversation

Pangoraw
Copy link
Collaborator

@Pangoraw Pangoraw commented Jul 13, 2024

not sure what we did here

fixes #1795.

Copy link
Contributor

Try this Pull Request!

Open Julia and type:

julia> import Pkg
julia> Pkg.activate(temp=true)
julia> Pkg.add(url="https://github.com/fonsp/Pluto.jl", rev="wat")
julia> using Pluto

@fonsp
Copy link
Owner

fonsp commented Aug 5, 2024

How is this PR going?

@disberd
Copy link
Contributor

disberd commented Aug 28, 2024

@fonsp, @Pangoraw,

What would be required still to test/merge this?
I tried to make a more recent PR on my fork to check if tests pass after latests changes in main and everything seems to pass (see disberd#327)

The amazing thing about this PR (for me at least) is that it would allow me to move most the evaluation I do in PlutoDevMacros to runtime rather than compile time, which will allow me to have progress bars and simplify quite a few things.
At the moment with everything happening at compile time, and with macro expansion not being reactive to logs (and not showing runtime in pluto) it can be sometime weird to wait for potentially long without anything clearly happening.

I still do not understand why,

but with the very small change in this PR using local modules work and it also respects exports that are added at runtime using Core.eval, as shown below:

ed7a8a53-d8f2-4fe1-a881-cad1b5a79b82.mp4

This is actually mind blowing 🎉

Notebook Code
# ╔═╡ d699cc81-bbd9-43f4-90f0-869a01a54d71
using PlutoUI

# ╔═╡ 79152609-ac03-4e55-8222-cb8329c06763
random_names = map(Symbol, 'a':'z') 

# ╔═╡ 8a20fbe5-9130-4e5c-8c80-a8b39d667c37
@bind wat Slider(random_names; show_value = true)

# ╔═╡ b7d4beda-6547-11ef-0a87-6bf0adbc0817
begin
	@eval module ASD end
	Core.eval(ASD, :($wat() = (; $wat = rand())))
	Core.eval(ASD, :(export $wat))
	using .ASD
end

# ╔═╡ 4a914dd9-7550-4fbe-b408-9feaeb8238aa
let
	m = @__MODULE__
	whatdefined = filter(n -> isdefined(m, n), random_names)
	@info "WAT" m wat whatdefined
	Core.eval(m, wat)()
end

@Pangoraw
Copy link
Collaborator Author

After giving it a new look, this seems like a sane approach.

  • For packages doing the reimport after moving variables will not affect the result since packages are not looked in the module environment.
  • This enable using .A globally because the A module is now moved before running the global using.

I would like to add tests for the behavior.

@Pangoraw Pangoraw marked this pull request as ready for review September 2, 2024 07:56
@Pangoraw Pangoraw merged commit 4da3a56 into main Sep 9, 2024
14 checks passed
@Pangoraw Pangoraw deleted the wat branch September 9, 2024 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

using .Module does not work anymore
3 participants