Skip to content

Fix module detection (for real this time)#23

Merged
MikeInnes merged 6 commits intomasterfrom
sp/fixmodulestuff
Feb 14, 2017
Merged

Fix module detection (for real this time)#23
MikeInnes merged 6 commits intomasterfrom
sp/fixmodulestuff

Conversation

@pfitzseb
Copy link
Member

@pfitzseb pfitzseb commented Jan 27, 2017

by using Tokenize.jl.

Should actually work this time around...

@marius311 Since you reported the bugs related to this, would you mind trying out this branch?

@marius311
Copy link
Contributor

Awesome, thanks for this. I'll play around with it this weekend and report back.

@MikeInnes
Copy link
Member

Is this good to go as far as you're concerned? Would be great to get this in the next release.

@sglyon
Copy link
Contributor

sglyon commented Feb 13, 2017

Can we add some test to see if annotating loops works?

I frequently have issues with code like this (forget the pointless code that doesn't do anything):

module Mod

    function foo(n)
        x = zeros(n)
        @inbounds for i 1:n
            x = sin(x)
        end
    end

end  # module

When I don't have @inbounds, it works fine. When I do have it Juno thinks the module ends when the @inbounds for does

@pfitzseb
Copy link
Member Author

I'm 99% sure this works now, but adding that test would be awesome regardless (maybe you could do that, @sglyon?).

@marius311
Copy link
Contributor

Sorry I never got back on this, having just checked I confirm it looks like it resolves everything I mentioned in JunoLab/atom-julia-client#308, as well as @sglyon's code from above. I also haven't found any other new problems.

JunoLab/atom-julia-client#198 however is still unresolved however, although that's more block detection as opposed to module detection I suppose.

@MikeInnes
Copy link
Member

Yup, block detection happens on the Atom/JS side. Will take a quick look at that.

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.

4 participants