-
Notifications
You must be signed in to change notification settings - Fork 4
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
Formal arg with ellipsis #7
base: master
Are you sure you want to change the base?
Formal arg with ellipsis #7
Conversation
Oh, shoot. I was planning to open this as a draft PR. Nevertheless, I would like to gather early feedback since I think it will be great to know the best practices from you. 😄 |
1b900c2
to
dc7ba6f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pull request! Your code seems correct, but it doesn't appear to work for me, unfortunately, likely due to a subtle issue with separated_list_partial()
unintentionally hard bailing due to it consuming the trailing ,
and failing to parse the subsequent ellipsis.
You should be able to reproduce the failure by running the following command:
echo '{ foo, ... }: foo' | cargo run -p nix-parser --example viewer
map(preceded(tokens::comma, tokens::ellipsis), |ellipsis| { | ||
Some(ellipsis) | ||
}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the closure could just as easily be replaced with:
map(preceded(tokens::comma, tokens::ellipsis), Some),
By the way, commit c807b89 has updated |
0cf33fd
to
e1711bd
Compare
1562aa6
to
c3b2a03
Compare
8175c19
to
3fc0c94
Compare
38b9cae
to
d5019aa
Compare
This PR will