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

Automatically wrap up the long code in the Report #169

Closed
Tracked by #4
Polkas opened this issue Nov 19, 2022 · 5 comments · Fixed by #218
Closed
Tracked by #4

Automatically wrap up the long code in the Report #169

Polkas opened this issue Nov 19, 2022 · 5 comments · Fixed by #218
Assignees
Labels

Comments

@Polkas
Copy link
Contributor

Polkas commented Nov 19, 2022

@nikolas-burkoff I think I found the solution to automatically wrap up the R code in the Report.
We need the formatR package and additional knitr global option. We could easily add this option as now we support global knitr ones.

knitr::opts_chunk$set(tidy.opts = list(width.cutoff = 60), tidy = TRUE)

or tidy = "styler"

Source: https://stackoverflow.com/a/74502546/5442527

@averissimo
Copy link
Contributor

averissimo commented Aug 3, 2023

Guideline for optimal line length: https://baymard.com/blog/line-length-readability

@chlebowa
Copy link
Contributor

chlebowa commented Aug 3, 2023

Acceptance criteria:

Figure out how to neatly wrap code in all report formats available.

@m7pr
Copy link
Contributor

m7pr commented Sep 25, 2023

Should we change default global knitr options in here?

renderRmd = function(blocks, yaml_header, global_knitr = list()) {

that are passed here
"\n```{r setup, include=FALSE}\nknitr::opts_chunk$set(%s)\n```\n",

or make it more explicit for the app developer, that he can make this change in there?

Users can always change .Rmd with the report file on their own, once it's downloaded, so I don't think this is a big deal.

@kartikeyakirar
Copy link
Contributor

kartikeyakirar commented Sep 26, 2023

knitr::opts_chunk$set(tidy.opts = list(width.cutoff = 60), tidy = TRUE)

This has dependencies for formatR, but the formatting looks good for other formats. Should we add it for all formats or just for PDF?

Should we change default global knitr options in here?

I believe this is a good approach if we're applying it to all formats.if not , based on the report type, we could specifically address changes for the PDF format.

@kartikeyakirar kartikeyakirar self-assigned this Sep 26, 2023
@m7pr
Copy link
Contributor

m7pr commented Sep 26, 2023

nah, just put it in the default global_knitr options and this will be applied to every format

kartikeyakirar added a commit that referenced this issue Oct 4, 2023
this Fixes
#169

I've implemented a solution that relies on the formatR package. I've
ensured that this solution checks for the package in the namespace; if
it's not accessible, the user will be notified. To maintain flexibility,
I've included the formatR package in the suggested dependencies, as
there are no strict dependencies on it.

In response to the feedback provided by @m7pr in [this
comment](#169 (comment)),
I've updated the default value of global_knitr to list(echo = TRUE,
tidy.opts = list(width.cutoff = 60), tidy = FALSE).

Please note that there was an alternative option to set tidy = "styler",
but I opted against it due to the additional dependency it would
introduce (the styler package). Feel free to review the changes and let
me know if any further adjustments are needed.

----
This branch utilizes the functionality provided by the 'formatR' package
to automatically add spaces and indentations to the code. However, it
does create a strong dependency on the 'formatR' package. If the package
is not installed, the output will be generated without any formatting.

Additionally, this branch introduces the following changes:

- It exposes Knitr options to users through the functions
'download_report_button_srv', 'reporter_previewer_srv,'
'simple_reporter_srv,' and the R6 renderer.
- The Roxygen documentation has been updated to reflect these
modifications.

changes in reports are observed as follows.

branch: main
<img width="613" alt="image"
src="https://github.com/insightsengineering/teal.reporter/assets/6700955/84998f72-1676-46bc-aff1-e376b68e2c22">

branch: 169_wrap_rcode@main
<img width="582" alt="image"
src="https://github.com/insightsengineering/teal.reporter/assets/6700955/be027ece-c2b8-45c0-85cc-719c7ee306f6">

---------

Signed-off-by: kartikeya kirar <kirar.kartikeya1@gmail.com>
Co-authored-by: kartikeya <kartikeya.kirar@unicle.life>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Marcin <133694481+m7pr@users.noreply.github.com>
Co-authored-by: 27856297+dependabot-preview[bot]@users.noreply.github.com <27856297+dependabot-preview[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants