Skip to content
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

Rtf rich text #184

Merged
merged 20 commits into from
Jul 2, 2023
Merged

Rtf rich text #184

merged 20 commits into from
Jul 2, 2023

Conversation

BrianLang
Copy link
Collaborator

@BrianLang BrianLang commented Jun 30, 2023

I've made the updates to remove dependencies and also resolved an issue with brace matching.
Previously nesting of braces mixing rtf_text code like {.emph text{^\\dagger}} would end up matching the wrong braces and producing unexpected rtf output. Now it correctly resolves pairs of braces with FILO.

closes #112
closes #185

@BrianLang BrianLang requested review from nanxstats and elong0527 July 1, 2023 05:50
@BrianLang BrianLang mentioned this pull request Jul 1, 2023
Copy link
Collaborator

@nanxstats nanxstats left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The patch looks neat! Thanks for improving on the previous implementation.

I only added a few minor fixes for code and documentation style to keep things consistent. Also used @noRd instead of @keyword internal to avoid generating .Rd files for internal functions, which is the existing convention in this package.

@elong0527 Any comments?

@BrianLang
Copy link
Collaborator Author

Thanks for the updates @nanxstats .
Do you have your own styler config? Or do you adapt these changes by hand? Agree that they look better.

@nanxstats
Copy link
Collaborator

Thanks for the updates @nanxstats . Do you have your own styler config? Or do you adapt these changes by hand? Agree that they look better.

Good question! styler can apply the general style guide but it is not aggressive (probably to be on the safe side). For example, it doesn't know if new lines should be added or removed, and if words are properly stylized under the context.

In those cases, I just apply the more comprehensive guides from the original tidyverse style guide manually when appropriate. That is ok because I think clean code / code style should be decided by humans ultimately, although automated rules surely help reduce manual work.

@elong0527
Copy link
Collaborator

Looks great! Thanks a lot for handling the task.

@elong0527 elong0527 mentioned this pull request Jul 2, 2023
@nanxstats nanxstats merged commit 9d5e660 into master Jul 2, 2023
@nanxstats nanxstats deleted the rtf_rich_text branch July 2, 2023 07:13
@BrianLang
Copy link
Collaborator Author

Thanks @nanxstats and @elong0527

@nanxstats
Copy link
Collaborator

@BrianLang This sounds like a excellent topic for an external blog post or vignette. It's super ✨visual✨

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.

Pkgdown fails on Master Add inline text format
3 participants