Skip to content

Conversation

@pminten
Copy link

@pminten pminten commented Aug 24, 2013

This fixes #1646

I'm not 100% happy with this change, it adds one list traversal in the common case and another in the uncommon case (improper list). However I couldn't see a better way. I suspect a more elegant solution would involve rewriting do_splice to not use a reversed list as input.

Copy link
Member

Choose a reason for hiding this comment

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

Can we just apply this change if Q#elixir_quote.escape is true?

Copy link
Member

Choose a reason for hiding this comment

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

I am wondering, maybe instead of reverse_improper returning two different results, could it simply the list with { '|', [], [left, right] } which we then simply pass to do_splice? Thanks!

Copy link
Author

Choose a reason for hiding this comment

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

That was my first attempt. But do_splice would quote that and you can't force it to unquote because the unquote option might not be on.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, perfect. :)

@pminten
Copy link
Author

pminten commented Aug 25, 2013

Updated to only make the new code trigger when escape is used.

josevalim pushed a commit that referenced this pull request Aug 25, 2013
@josevalim josevalim merged commit f02f90f into elixir-lang:master Aug 25, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Macro.escape doesn't handle HashDicts

2 participants