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.

Sign up for GitHub

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

Jump to bottom

Expanding with unused empty array values #88

Closed
gerwitz opened this issue May 9, 2018 · 4 comments · Fixed by #92
Closed

Expanding with unused empty array values #88

gerwitz opened this issue May 9, 2018 · 4 comments · Fixed by #92
Assignees
@namusyaka
Labels
bug

Comments

@gerwitz
Copy link

gerwitz commented May 9, 2018 •
edited
Loading

Passing in an unused value that is an empty array causes an entertaining failure.

Expander.expand() replaces the corresponding key with an empty array:

mustermann/mustermann/lib/mustermann/ast/expander.rb

Line 95 in 3409395

values = values.each_with_object({}){ |(key, value), new_hash|

Then the mapping throws an error, which is "handled" by Expander.error_for() which attempts to sort the keys, naturally causing "ArgumentError - comparison of Symbol with Array"
@gerwitz gerwitz changed the title Empty array values fail Expanding with unused empty array values May 9, 2018
@namusyaka
Copy link
Member

@gerwitz Could you provide minimal example for reproducing the failure?

@gerwitz
Copy link
Author

gerwitz commented Nov 3, 2018

It's been a while and I hadn't discovered the :ignore flag then, but I think this is what I was talking about:

p = Mustermann.new(':one:two')
p.expand({one: 1, two: "2", three: [], four: "four"})

Note how removing the :four item from that Hash leads to a very different exception.

@namusyaka
Copy link
Member

Thanks for the tips.
Definitely bug. I'll take a look soon.

@namusyaka namusyaka added the bug label Nov 3, 2018
@namusyaka namusyaka self-assigned this Nov 3, 2018
namusyaka added a commit to namusyaka/mustermann19 that referenced this issue Nov 3, 2018
@namusyaka
don't use adjusted values for reporting error
a22810b 
The previous code led regular error to be hidden and return meaningless ArgumentError.
In order to avoid that, this commit makes code not use adjusted values for
reporting error, use original values instead.

Fixes sinatra#88
namusyaka added a commit to namusyaka/mustermann19 that referenced this issue Nov 3, 2018
@namusyaka
don't use adjusted values for reporting error
6310751 
The previous code led regular error to be hidden and return meaningless ArgumentError.
In order to avoid that, this commit makes code not use adjusted values for
reporting error, use original values instead.

Fixes sinatra#88
@namusyaka namusyaka mentioned this issue Nov 3, 2018
Merged
@gerwitz
Copy link
Author

gerwitz commented Nov 3, 2018

👍 My tests get proper errors now.

@gerwitz gerwitz closed this as completed Nov 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@namusyaka namusyaka

Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

don't use adjusted values for reporting error namusyaka/mustermann19
2 participants
@gerwitz @namusyaka