Skip to content

Conversation

@rich-iannone
Copy link
Member

@rich-iannone rich-iannone commented Sep 7, 2022

This PR removes the few stringr functions that remained in the package. A handful of simple replacement functions are used instead.

@rich-iannone rich-iannone marked this pull request as ready for review September 7, 2022 12:07
@rich-iannone rich-iannone requested a review from cscheid September 7, 2022 12:07
@cderv
Copy link
Collaborator

cderv commented Sep 7, 2022

@rich-iannone Interested by this work for knitr too: yihui/knitr#1549

I don't know if all the functions are covered by your switch here or not, but definitely some overlap.

Only constraint for knitr - performance. We need to be sure that we don't loose performance in the rendering process. Maybe also an issue with gt

@cscheid cscheid merged commit d3d590c into master Sep 7, 2022
@rich-iannone rich-iannone deleted the stringr-replace branch September 7, 2022 16:18
@rich-iannone
Copy link
Member Author

@rich-iannone Interested by this work for knitr too: yihui/knitr#1549

I don't know if all the functions are covered by your switch here or not, but definitely some overlap.

Only constraint for knitr - performance. We need to be sure that we don't loose performance in the rendering process. Maybe also an issue with gt

@cderv Many of the stringr functions are covered here by the replacements. I could definitely do the same for knitr, not a problem. A lot of the replacements in this PR are concentrated in one part of the code path (inlining styles in HTML tags). It's not any slower and that whole section will be replaced soon anyway with a fast JS-mediated function.

@cderv
Copy link
Collaborator

cderv commented Sep 7, 2022

Many of the stringr functions are covered here by the replacements. I could definitely do the same for knitr, not a problem.

Awesome !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants