Skip to content
This repository has been archived by the owner on Jun 9, 2023. It is now read-only.

fix typo #72

Merged
merged 5 commits into from
Jan 18, 2017
Merged

fix typo #72

merged 5 commits into from
Jan 18, 2017

Conversation

ZacLN
Copy link
Contributor

@ZacLN ZacLN commented Jan 17, 2017

No description provided.

@ZacLN ZacLN mentioned this pull request Jan 17, 2017
@ararslan
Copy link
Member

Can you add a test that catches this case?

@ZacLN
Copy link
Contributor Author

ZacLN commented Jan 17, 2017

I'm not actually sure what the expected behaviour is here. With Base.parse this parses to
:([$(Expr(:parameters, 3));1;2]) and only errors on evaluation, do we mimic the parsing?

@ZacLN
Copy link
Contributor Author

ZacLN commented Jan 17, 2017

i.e. will
@test Parser.parse("[1,2;3]") == :([$(Expr(:parameters, 3));1;2])
suffice?

@ararslan
Copy link
Member

I think the goal of this package is to mimic the base parsing as closely as possible, so I think your suggested test will do just fine.

@KristofferC, do you have any recommendations regarding how this should be tested?

@@ -1022,3 +1022,6 @@ facts("misc syntax changes") do
@fact (Parser.parse(ex) |> norm_ast) --> (Base.parse(ex) |> norm_ast)
end
end

# issue #72 : failure to parse and incorrectly specified vector
Base.@test Parser.parse("[1,2;3]") == :([$(Expr(:parameters, 3));1;2])
Copy link
Member

Choose a reason for hiding this comment

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

Actually, looking a few lines above this, I think we can follow suit with existing tests and simplify this to

Base.@test Parser.parse("[1,2;3]") == Base.parse("[1,2;3]")

Though since the rest of the tests are using FactCheck, this should probably use the @fact a --> b syntax rather than Base.@test.


#issue 72
facts("parse an incorrectly specified vector") do
@fact Parser.parse("[1,2;3]") --> :([$(Expr(:parameters, 3));1;2])
Copy link
Member

Choose a reason for hiding this comment

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

Should use Base.parse("[1,2,;3]") on the RHS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes!

@ararslan ararslan merged commit f15bb47 into JuliaLang:master Jan 18, 2017
@ZacLN ZacLN deleted the patch-1 branch January 18, 2017 06:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants