-
Notifications
You must be signed in to change notification settings - Fork 70
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
Implement tag()
in C
#416
Open
mgirlich
wants to merge
1
commit into
rstudio:main
Choose a base branch
from
mgirlich:tag-in-c
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Implement tag()
in C
#416
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
#include "utils.h" | ||
|
||
SEXP have_name(SEXP x) { | ||
SEXP nms = PROTECT(Rf_getAttrib(x, R_NamesSymbol)); | ||
R_xlen_t n = Rf_xlength(x); | ||
SEXP out = PROTECT(Rf_allocVector(LGLSXP, n)); | ||
|
||
if (nms == R_NilValue) { | ||
for (R_xlen_t i = 0; i < n; ++i) { | ||
SET_LOGICAL_ELT(out, i, 0); | ||
} | ||
} else { | ||
for (R_xlen_t i = 0; i < n; ++i) { | ||
SEXP nm_i = STRING_ELT(nms, i); | ||
SET_LOGICAL_ELT(out, i, nm_i != NA_STRING & nm_i != chr_empty); | ||
} | ||
} | ||
|
||
UNPROTECT(2); | ||
return out; | ||
} | ||
|
||
SEXP new_tag(SEXP tagName, SEXP varArgs, SEXP noWS, SEXP renderHook) { | ||
R_xlen_t n = Rf_xlength(varArgs); | ||
|
||
// TODO validate that varArgs is a list | ||
|
||
// Unnamed arguments are flattened and added as children. | ||
// Named arguments become attribs, dropping NULL and length-0 values | ||
SEXP namedFlag = PROTECT(have_name(varArgs)); | ||
|
||
// Calculate number of attributes and children | ||
R_xlen_t n_attributes = 0; | ||
R_xlen_t n_children = n; | ||
for (R_xlen_t i = 0; i < n; ++i) { | ||
int arg_i_empty = Rf_xlength(VECTOR_ELT(varArgs, i)) == 0; | ||
n_attributes = n_attributes + (arg_i_empty ? 0 : LOGICAL_ELT(namedFlag, i)); | ||
n_children = n_children - LOGICAL_ELT(namedFlag, i); | ||
} | ||
|
||
// Create attributes and children | ||
SEXP varArgNms = Rf_getAttrib(varArgs, R_NamesSymbol); | ||
SEXP attributes = PROTECT(Rf_allocVector(VECSXP, n_attributes)); | ||
SEXP attribute_nms = PROTECT(Rf_allocVector(STRSXP, n_attributes)); | ||
Rf_setAttrib(attributes, R_NamesSymbol, attribute_nms); | ||
|
||
SEXP children = PROTECT(Rf_allocVector(VECSXP, n_children)); | ||
R_xlen_t i_attributes = 0; | ||
R_xlen_t i_children = 0; | ||
|
||
for (R_xlen_t i = 0; i < n; ++i) { | ||
SEXP arg_i = VECTOR_ELT(varArgs, i); | ||
bool arg_i_empty = Rf_xlength(arg_i) == 0; | ||
if (LOGICAL_ELT(namedFlag, i)) { | ||
if (!arg_i_empty) { | ||
SET_VECTOR_ELT(attributes, i_attributes, arg_i); | ||
SEXP arg_i_nm = STRING_ELT(varArgNms, i); | ||
SET_STRING_ELT(attribute_nms, i_attributes, arg_i_nm); | ||
++i_attributes; | ||
} | ||
} else { | ||
SET_VECTOR_ELT(children, i_children, arg_i); | ||
++i_children; | ||
} | ||
} | ||
|
||
// Create tag | ||
R_xlen_t n_fields = 3; | ||
if (noWS != R_NilValue) { | ||
++n_fields; | ||
} | ||
if (renderHook != R_NilValue) { | ||
++n_fields; | ||
} | ||
SEXP tag = PROTECT(Rf_allocVector(VECSXP, n_fields)) ; | ||
SEXP field_nms = PROTECT(Rf_allocVector(STRSXP, n_fields)); | ||
Rf_setAttrib(tag, R_NamesSymbol, field_nms); | ||
Rf_classgets(tag, tag_class); | ||
|
||
SET_VECTOR_ELT(tag, 0, tagName); | ||
SET_STRING_ELT(field_nms, 0, chr_name); | ||
SET_VECTOR_ELT(tag, 1, attributes); | ||
SET_STRING_ELT(field_nms, 1, chr_attribs); | ||
SET_VECTOR_ELT(tag, 2, children); | ||
SET_STRING_ELT(field_nms, 2, chr_children); | ||
|
||
R_xlen_t field_i = 3; | ||
// Conditionally include the `.noWS` field. | ||
// We do this to avoid breaking the hashes of existing tags that weren't leveraging .noWS. | ||
if (noWS != R_NilValue) { | ||
SET_VECTOR_ELT(tag, field_i, noWS); | ||
SET_STRING_ELT(field_nms, field_i, chr_nows); | ||
++field_i; | ||
} | ||
// Conditionally include the `.renderHooks` field. | ||
// We do this to avoid breaking the hashes of existing tags that weren't leveraging .renderHooks. | ||
if (renderHook != R_NilValue) { | ||
SET_STRING_ELT(field_nms, field_i, chr_renderhooks); | ||
if (TYPEOF(renderHook) == VECSXP) { | ||
SET_VECTOR_ELT(tag, field_i, renderHook); | ||
} else { | ||
SEXP renderHookList = PROTECT(Rf_allocVector(VECSXP, 1)); | ||
SET_VECTOR_ELT(renderHookList, 0, renderHook); | ||
SET_VECTOR_ELT(tag, field_i, renderHookList); | ||
UNPROTECT(1); | ||
} | ||
} | ||
|
||
UNPROTECT(6); | ||
return tag; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
#include "utils.h" | ||
|
||
SEXP tag_class = NULL; | ||
|
||
SEXP chr_empty = NULL; | ||
|
||
SEXP chr_name = NULL; | ||
SEXP chr_attribs = NULL; | ||
SEXP chr_children = NULL; | ||
SEXP chr_nows = NULL; | ||
SEXP chr_renderhooks = NULL; | ||
|
||
void htmltools_initialize_utils(SEXP ns) { | ||
tag_class = Rf_allocVector(STRSXP, 1); | ||
R_PreserveObject(tag_class); | ||
SET_STRING_ELT(tag_class, 0, Rf_mkChar("shiny.tag")); | ||
|
||
R_PreserveObject(chr_empty = Rf_mkChar("")); | ||
|
||
R_PreserveObject(chr_name = Rf_mkChar("name")); | ||
R_PreserveObject(chr_attribs = Rf_mkChar("attribs")); | ||
R_PreserveObject(chr_children = Rf_mkChar("children")); | ||
R_PreserveObject(chr_nows = Rf_mkChar(".noWS")); | ||
R_PreserveObject(chr_renderhooks = Rf_mkChar(".renderHooks")); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
#ifndef HTMLTOOLS_UTILS_H | ||
#define HTMLTOOLS_UTILS_H | ||
|
||
#define R_NO_REMAP | ||
#include <R.h> | ||
#include <Rinternals.h> | ||
#include <stdbool.h> | ||
|
||
extern SEXP tag_class; | ||
|
||
extern SEXP chr_empty; | ||
|
||
extern SEXP chr_name; | ||
extern SEXP chr_attribs; | ||
extern SEXP chr_children; | ||
extern SEXP chr_nows; | ||
extern SEXP chr_renderhooks; | ||
|
||
#endif |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -386,6 +386,7 @@ test_that("Old tags predating rlang::list2 can still be rendered", { | |
}) | ||
|
||
test_that("tag with noWS works",{ | ||
skip("should tag accept only lists?") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. see above |
||
oneline <- tag("span", list(tag("strong", "Super strong", .noWS="outside"))) | ||
expect_identical(as.character(oneline), "<span><strong>Super strong</strong></span>") | ||
}) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
It is documented that
varArgs
must be a list but there is a test where this is not the case. Should we allowvarArgs
to be e.g. a character and simply wrap it in a list?