-
-
Notifications
You must be signed in to change notification settings - Fork 219
Update Attributes.R (closes #1017) #1016
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
Conversation
Corrected parsing of multiple "depends" statements in "cppFunction".
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.
Good catch! LGTM.
No worries. Thanks to all for producing such a useful package! |
@tjmckinley while you're at it... Mind fixing the code for Lines 258 to 259 in 975361f
|
@tjmckinley Thank you. Please also consider reading Contributing before sending PRs. What is the motivation for the PR? What it fixes is not a defect but an aesthetic preference. Which would change something used as it currently is and was by lots of people. We generally don't do that without good reason. |
My apologies @eddelbuettel. I will log an issue now with a reprex. |
Hi @eddelbuettel. I've logged an issue (#1017) with a reprex. It's not purely aesthetic in this case since the code won't compile for the example since the header file for
produces code with scaffolding:
with no Many thanks, TJ |
By the way, many thanks @eddelbuettel, @coatless for producing this fantastic package. I can't do without it! Apologies also if I take a while to reply. I'm in Japan and hence the time difference causes a lag in response time. |
Yes @coatless. Happy to do this if you and @eddelbuettel think this is an issue. |
LGTM as well. |
Looks good to me too after a closer look; merging. |
When passing multiple "depends" arguments to "cppFunction" e.g.
this was producing code with scaffolding:
rather than