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

Add copy to clipboard (#74) and markdown formatting (#78) features to makeOxygen() #79

Merged
merged 19 commits into from
Feb 27, 2024

Conversation

elipousson
Copy link
Contributor

Also:

  • Replace use of writeLines() when printing output with cli_code()
  • Minor fixes to DESCRIPTION by removing miniUI + shiny from Imports + adding "Encoding: UTF-8"

Also replaces superseded `@return` tag with updated `@returns` + replaces `writeLines()` with `cli_code()`

Also update DESCRIPTION by removing miniUI + shiny from Imports (also listed in Suggests), adds rlang to Imports, and add "Encoding: UTF-8"
@elipousson
Copy link
Contributor Author

If this is a welcome pull request, it may be helpful to add an option to set the use_md parameter globally. Happy to add that in but wanted to check first.

@yonicd
Copy link
Owner

yonicd commented Aug 7, 2023

thanks for the PR. I will review it shortly

- rename use_md to markdown for consistency w/ usethis
- drop rlang from imports
- add helper functions obj_lbl + prep_fn_roxy + prep_tbl_roxy + set_roxy_header
Also swamp message for cli functions
- Fix issue w/ substitute handling of env
- Add use_labels parameter
Copy link

codecov bot commented Feb 25, 2024

Codecov Report

Attention: Patch coverage is 78.85714% with 37 lines in your changes are missing coverage. Please review.

Project coverage is 35.53%. Comparing base (3d4acbb) to head (bc15033).

Files Patch % Lines
R/makeOxyFile.R 65.38% 9 Missing ⚠️
R/makeOxygen.R 92.91% 9 Missing ⚠️
R/makeDictionary.R 0.00% 7 Missing ⚠️
R/pretty_utils.R 22.22% 7 Missing ⚠️
R/makeImport.R 0.00% 3 Missing ⚠️
R/opts_complete.R 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master      #79       +/-   ##
===========================================
+ Coverage   22.00%   35.53%   +13.53%     
===========================================
  Files          28       28               
  Lines        1177     1255       +78     
===========================================
+ Hits          259      446      +187     
+ Misses        918      809      -109     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yonicd
Copy link
Owner

yonicd commented Feb 25, 2024

Gha shows that {crayon} is missing from desc file. There is still a crayon::red somewhere hanging around that is being picked up in the ns.

@elipousson
Copy link
Contributor Author

I’ll take a look and also fix the test coverage issues! The scope of this PR expanded a good bit beyond my initial draft back in August when I got back to it last night so, if you want some but not all of the code, just let me know and I can pull those changes back out again.

Also add make_oxygen alias
Also

- avoids copying code from each file to clipboard
- adds make_oxy_file alias
- invisibly returns file list
- cleans up transition to cli
Add tests for makeOxyFile + makeOxygen
Also add dir.out argument (using same arg name as untangle) + expand tests
@elipousson
Copy link
Contributor Author

I think I got the code coverage issues fixed.

The main additions beyond the original additional of auto-copying with {clipr} and markdown formatting with {roxygen2md} include:

  • Refactoring makeOxygen() with new helper functions, new parameters for title/description, and the option to pull label parameters for data frame columns (enabled using the use_labels parameter), and adding support for vector data objects
  • Refactoring makeOxyFile() to improve support for directory inputs and add dir.out parameter and to expose the new parameters added to makeOxygen()
  • Convert error/informational messages to work with the cli package
  • Update makeDictionary() to handle file as well as directory input paths
  • Update tests and documentation to match the modified functions and add snake case-style aliases for several functions

I use sinew all the time and I'd love to get these changes into the package if possible. Let me know if any additional changes are needed!

@yonicd
Copy link
Owner

yonicd commented Feb 26, 2024

@elipousson it is great to hear that you are using the package!

I think the only thing left is to bump the version. Enough changes have been made to move from 0.4 to 0.5. After you change that I'll merge it into master.

Thanks for all the contributions!

@yonicd yonicd merged commit 0a5b6ac into yonicd:master Feb 27, 2024
5 checks passed
@yonicd
Copy link
Owner

yonicd commented Feb 27, 2024

@elipousson thanks again for all the contributions and hard work of thinking how to refactor the package.

@elipousson
Copy link
Contributor Author

@yonicd My pleasure! I have a few additional tweaks in mind but didn't want to crowd an already crowded PR after you had already signed off. Time permitting, I may open a new GitHub issue with those ideas in the next few weeks.

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.

2 participants