-
Notifications
You must be signed in to change notification settings - Fork 26
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
New argument to provide additional options to tikzpicture environment #148
base: master
Are you sure you want to change the base?
Conversation
Looks reasonable to me. Cc @krlmlr |
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. Please let me know if you're still interested in this.
@@ -315,6 +317,7 @@ static Rboolean TikZ_Setup( | |||
tikzInfo->onefile = onefile; | |||
tikzInfo->timestamp = timestamp; | |||
tikzInfo->verbose = verbose; | |||
tikzInfo->pictureOptions = calloc_strcpy(pictureOptions); |
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.
Do we need to free this after we're done?
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.
We might need a call to free()
that corresponds to the calloc_strcpy()
call.
@@ -172,6 +172,7 @@ SEXP TikZ_StartDevice ( SEXP args ){ | |||
int maxSymbolicColors = asInteger(CAR(args)); args = CDR(args); | |||
Rboolean timestamp = asLogical(CAR(args)); args = CDR(args); | |||
Rboolean verbose = asLogical(CAR(args)); args = CDR(args); | |||
const char *pictureOptions = CHAR(asChar(CAR(args))); args = CDR(args); |
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'm slightly worried about protection here, because asChar()
might allocate memory in corner cases.
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.
Maybe CHAR(PROTECT(asChar(...)))
, and then increase the number passed to UNPROTECT()
by one?
Yes, I'm still interested in this, and I thought that I checked this PR now and then – apparently not, since I did not see your question from August 2017, sorry. I just got a notification about the closing of this PR and would be interested in re-opening it. Concerning your questions, I'm not a C/C++ expert and am afraid that I cannot answer your memory-related questions competently. Are you willing to continue? |
Sure, I can reopen it. I have just pushed an update to tikzDevice to CRAN, but it hasn't been accepted yet. After that I'll be looking for a new maintainer who will be more responsive hopefully ;-) |
Could you please take a look at the conflict? |
* Added an argument pictureOptions to the R function `tikz`. * Added the argument to the C function `TikZ_Setup` and the structure tikzDevDesc. * Made use of the argument, if provided, in the the C function `TikZ_StartDevice`. * Added R documentation and rebuild documentation with roxygen. * Added argument to vignette (in sec. 3.2 'Usage')
326e72a
to
a6e8eed
Compare
I've rebased my changes. It has no conflicts anymore and we can start tackling the questions you raised above. If you think that things should change, feel free to make these changes. |
I am sorry that this has been lying around for so long. Can you please rebase the PR again? Thanks. |
The changes in this pull request extend the R-function
tikz
with a new character string argumentpictureOptions
that allows for a specification of additional options that are provided to thetikzpicture
environment's option list.This feature results from a personal need of mine. I need the plots from
tikzdevice
to be typeset in a sans-serif font family. So far I have generated the plots as complete figures and have manually added,font=\sffamily
to the option list of thetikzpicture
environment.Using the new argument it is now possible to add the option from within R
which then results in the following TikZ/LaTeX code:
Of course other options, or lists of options, can be specified as well.