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

for outer loops #130

Merged
merged 4 commits into from
Dec 8, 2017
Merged

for outer loops #130

merged 4 commits into from
Dec 8, 2017

Conversation

pfitzseb
Copy link
Collaborator

Fixes #129.

outer is only a keyword in for outer loops, right?

@sglyon
Copy link
Contributor

sglyon commented Nov 25, 2017

I didn't even know that outer is ever a keyword.

Anyone else willing to review this?

@pfitzseb
Copy link
Collaborator Author

Well, it's 0.7 only, and I vaguely remember that there was a PR that added it. Can't really be bothered to look for it though, so tagging @ararslan again :)

@ararslan
Copy link
Member

ararslan commented Nov 25, 2017

outer is only a keyword in for outer loops, right?

Correct, though it can annotate any variable in the loop declaration, e.g.

i = j = 1
for outer i = 1:10, outer j = 3:4
    # ...
end

It is indeed 0.7 only, added in JuliaLang/julia#22659.

@pfitzseb
Copy link
Collaborator Author

That case is surprisingly difficult to handle properly...

@ararslan
Copy link
Member

You may be able to take a look at how the Vim plugin does it; I think Carlo figured it out somehow.

even on multiple lines :)
@pfitzseb
Copy link
Collaborator Author

pfitzseb commented Dec 5, 2017

Okay, this should be mostly fine now.
Only thing I'd like to improve is the end regex ((?<!,)(\\n)) -- it would be awesome if we could use something like (?<!,)(\\s*\\n), but that gives false positives for e.g. ", \n" because the \s* isn't greedy enough.

and better specs
@pfitzseb
Copy link
Collaborator Author

pfitzseb commented Dec 5, 2017

Alright, should really work now and handle most cases properly :)

@pfitzseb
Copy link
Collaborator Author

pfitzseb commented Dec 7, 2017

Barring any objections I'll merge this tomorrow(-ish).

@pfitzseb pfitzseb merged commit e457bbd into master Dec 8, 2017
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.

3 participants