Use roxygen2 parser for roxy comments + lightparser for figure and tbl captions #29
Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
WalkthroughThe updates introduce several enhancements and new functionalities across multiple R scripts, focusing on file navigation, Roxygen tag parsing, and outline generation in RStudio. Key changes include adding functions for navigating active documents, refining Roxygen comment handling, improving outline criteria, and updating the logic for file renaming and screenshot functionalities. These updates aim to bolster interactive sessions, streamline project management, and enhance user feedback during document processing. Changes
Sequence Diagram(s) (Beta)No sequence diagrams are generated for these changes as they are too varied and specific to individual functionalities. Possibly related issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Out of diff range and nitpick comments (8)
R/use-todo.R (2)
66-66: Add a brief description of the function's purpose in the documentation.Consider adding a brief description of what the function
complete_tododoes at the beginning of its documentation to improve clarity for users.
155-155: Clarify the purpose of thecheck_referenced_filesfunction call.Consider adding a comment explaining why
check_referenced_filesis called here, especially since it's conditional on theactionnot being "test".R/rename-files.R (1)
155-155: Clarify the conditional logic for callingcheck_referenced_files.Consider adding a comment to explain the conditions under which
check_referenced_filesis called. This will help future maintainers understand the decision-making process behind this function call.README.md (5)
Line range hint
22-22: Consider using a synonym for "hard" to enhance the expression.
Line range hint
48-48: Change "recommended to avoid surprises" to "recommended for avoiding surprises" to align with grammatical norms.- recommended to avoid surprises + recommended for avoiding surprises
Line range hint
52-52: The phrase "human readable" should be hyphenated to "human-readable" when used as an adjective.- human readable paths + human-readable paths
Line range hint
54-54: Consider replacing "in the long run" with a more concise alternative like "eventually".- in the long run + eventually
Line range hint
63-63: Clarify the usage of "to-do" with a hyphen when referring to tasks.- use_todo() + use_to-do()
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (18)
- DESCRIPTION (1 hunks)
- R/escape-inline-markup.R (1 hunks)
- R/open.R (1 hunks)
- R/outline-criteria.R (3 hunks)
- R/outline-roxy.R (1 hunks)
- R/outline.R (10 hunks)
- R/rename-files.R (2 hunks)
- R/use-todo.R (5 hunks)
- README.md (2 hunks)
- TODO.R (1 hunks)
- man/complete_todo.Rd (1 hunks)
- tests/testthat/_ref/my-analysis.R (2 hunks)
- tests/testthat/_ref/test-roxygen-safeguard.R (1 hunks)
- tests/testthat/_ref/test-roxygen.R (1 hunks)
- tests/testthat/_snaps/use-todo.md (1 hunks)
- tests/testthat/test-outline-roxy.R (1 hunks)
- tests/testthat/test-outline.R (1 hunks)
- tests/testthat/test-use-todo.R (4 hunks)
Files skipped from review due to trivial changes (5)
- DESCRIPTION
- R/escape-inline-markup.R
- TODO.R
- tests/testthat/_ref/test-roxygen-safeguard.R
- tests/testthat/test-outline-roxy.R
Additional comments not posted (15)
tests/testthat/_snaps/use-todo.md (1)
4-4: Parameter name updated tolinecorrectly reflects the changes in the function definition.man/complete_todo.Rd (2)
7-7: Documentation updated to reflect the parameter name change fromline_idtoline.
10-10: Argument descriptions updated correctly to reflect the new parameter names and functionalities.tests/testthat/_ref/my-analysis.R (1)
3-3: Addedsilent = TRUEto suppress warnings during file reading, which is a good practice for handling potentially problematic CSV files.tests/testthat/test-outline.R (1)
54-54: Handling ofbefore_and_after_emptyimproved to prevent errors and ensure correct behavior when the parameter is not explicitly set.tests/testthat/_ref/test-roxygen.R (1)
1-102: The file is well-structured with clear examples of roxygen comments, which is crucial for testing the parsing and handling of these comments in documentation.R/outline-roxy.R (2)
15-132: Significant refactoring inextract_roxygen_tag_locationimproves handling of roxygen blocks. Effective use ofpurrrandroxygen2functions demonstrates good functional programming practices.
134-225:join_roxy_funeffectively combines parsed roxygen blocks into a structured format. Commendable use oftidyranddplyrfor data manipulation.R/outline-criteria.R (2)
32-32: Updatedo_is_roxygen_commentto usestringr::str_detectfor more accurate detection of roxygen comments.
Line range hint
124-191: Extensive updates indefine_outline_criteriaimprove handling of roxygen comments and integration into outline criteria. Robust use ofrlangandpurrrfor error handling and data manipulation.R/use-todo.R (2)
80-80: Parameter renaming fromline_idtolineimproves clarity.The renaming of the parameter from
line_idtolinemakes the function's interface clearer and more intuitive.
82-82: Ensure consistent error handling and user feedback.Ensure that error handling and user feedback are consistent across the project. This can be verified by checking the usage of
cli::cli_abortandcli::cli_warnin other parts of the codebase.Also applies to: 91-91
Verification successful
Ensure Consistent Error Handling and User Feedback
The search results indicate that
cli::cli_abortandcli::cli_warnare used consistently across multiple files in the project for error handling and user feedback.
- Files with
cli::cli_abortandcli::cli_warnusage:
R/use-todo.RR/outline-roxy.RR/outline.RR/open.RR/escape-inline-markup.RR/utils-proj.RR/screenshot.RR/proj-list.RR/outdated-pkgs.RR/named.RR/files-conflicts.RR/dplyr-helpers.RR/dplyr-plus.RR/case-if-any.RR/rename-files.RR/browse-pkg.RThe usage of these functions appears to be widespread and consistent, suggesting that the project maintains a uniform approach to error handling and user feedback.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent error handling and user feedback across the project. # Test: Search for inconsistent error handling patterns. rg --type R $'cli::cli_abort|cli::cli_warn'Length of output: 70
Script:
#!/bin/bash # Description: Verify consistent error handling and user feedback across the project. # Test: Search for inconsistent error handling patterns in R files. rg --glob '*.R' 'cli::cli_abort|cli::cli_warn'Length of output: 6254
R/outline.R (3)
125-132: The changes in handling unsaved documents and improved path normalization are well-implemented.
368-369: The addition of detailed user feedback and error handling inproj_outlineenhances the robustness of the function.
Line range hint
454-471: The modifications to exclude certain directories and files from outlines indir_outlinefunction help in reducing clutter and improving usability.
R/open.R
Outdated
| # FIXME (upstream) the color div doesn't go all the way r-lib/cli#694 | ||
| "i" = paste(cli::col_grey("The deleted file"), "{.path {elems$full_path}}", cli::col_grey("contents are returned invisibly in case you need them.")) |
There was a problem hiding this comment.
Adjust the CLI color issue as noted in the FIXME comment.
- # FIXME (upstream) the color div doesn't go all the way r-lib/cli#694
+ # TODO: Monitor upstream issue r-lib/cli#694 for resolution and update accordinglyChange the FIXME comment to a TODO to monitor the upstream issue regarding the CLI color division. This keeps the codebase clean while acknowledging that the issue is recognized and being tracked.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| # FIXME (upstream) the color div doesn't go all the way r-lib/cli#694 | |
| "i" = paste(cli::col_grey("The deleted file"), "{.path {elems$full_path}}", cli::col_grey("contents are returned invisibly in case you need them.")) | |
| # TODO: Monitor upstream issue r-lib/cli#694 for resolution and update accordingly | |
| "i" = paste(cli::col_grey("The deleted file"), "{.path {elems$full_path}}", cli::col_grey("contents are returned invisibly in case you need them.")) |
There was a problem hiding this comment.
Actionable comments posted: 1
Out of diff range and nitpick comments (5)
README.md (5)
Line range hint
22-22: Consider using a more specific term than "hard" to describe the challenges.
Line range hint
48-48: The phrase "recommended to avoid surprises" could be rephrased for clarity.- recommended to avoid surprises + recommended to prevent unexpected issues
Line range hint
52-52: The term "human readable" is typically hyphenated when used as an adjective.- human readable paths + human-readable paths
Line range hint
54-54: Consider replacing "in the long run" with a more concise alternative like "eventually".- it can take away the pain in the long run + it can eventually reduce the pain
Line range hint
63-63: Clarify the usage of "to-do" as a noun with a hyphen.- use_todo() + use_to-do()
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- R/escape-inline-markup.R (1 hunks)
- R/outline-criteria.R (3 hunks)
- README.md (2 hunks)
Files not summarized due to errors (1)
- README.md: Error: Message exceeds token limit
Files skipped from review as they are similar to previous changes (2)
- R/escape-inline-markup.R
- R/outline-criteria.R
Additional Context Used
LanguageTool (5)
README.md (5)
Near line 22: To elevate your writing, try using a synonym here.
Context: ...tions across projects. Sometimes, it is hard to do that. reuseme also aims to help m...
Near line 48: The verb ‘recommended’ is used with the gerund form.
Context: ...on controlled with git (optional, but recommended to avoid surprises! No need to be hosted on repo...
Near line 52: This word is normally spelled with a hyphen.
Context: ...igned on Windows) - You use machine and human readable paths (i.e. no spaces, special charac...
Near line 54: ‘in the long run’ might be wordy. Consider a shorter alternative.
Context: ...your files, it can take away the pain in the long run! To take advantage of reuseme, it is h...
Near line 63: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...ns likeproj_switch(),proj_list(),use_todo()to be optimized. ## Example Since ...
Merge branch 'main' into roxy-parse # Conflicts: # TODO.R
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- R/todo.R (1 hunks)
- TODO.R (1 hunks)
- tests/testthat/test-todo.R (1 hunks)
Files skipped from review due to trivial changes (2)
- R/todo.R
- tests/testthat/test-todo.R
Files skipped from review as they are similar to previous changes (1)
- TODO.R
…roxygen comments for now (will probably merge in the future PR.)
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (5)
README.md (5)
Line range hint
22-22: Consider using a synonym for "hard" to enhance the tone of the sentence.
Line range hint
48-48: Use the gerund form "recommending" instead of "recommended" to improve grammatical correctness.
Line range hint
52-52: Add a hyphen to "human-readable" for correct spelling.
Line range hint
54-54: Consider a more concise alternative to "in the long run," such as "eventually."
Line range hint
63-63: Clarify whether "to-do" (noun) or "to do" (verb) is intended here to avoid confusion.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- R/outline-criteria.R (6 hunks)
- R/outline-roxy.R (1 hunks)
- R/outline.R (5 hunks)
- README.md (9 hunks)
- tests/testthat/_ref/test-roxygen.R (1 hunks)
- tests/testthat/_snaps/outline-criteria.md (1 hunks)
- tests/testthat/_snaps/outline.md (1 hunks)
- tests/testthat/test-outline-criteria.R (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- R/outline-roxy.R
- R/outline.R
- tests/testthat/_ref/test-roxygen.R
Additional Context Used
Markdownlint (5)
tests/testthat/_snaps/outline-criteria.md (1)
16: Expected: 1; Actual: 2
Multiple consecutive blank linestests/testthat/_snaps/outline.md (4)
88: Expected: 1; Actual: 2
Multiple consecutive blank lines
38: null
Multiple top-level headings in the same document
49: null
Multiple top-level headings in the same document
62: null
Multiple top-level headings in the same document
Additional comments not posted (4)
tests/testthat/test-outline-criteria.R (1)
35-36: The test foro_is_tab_plot_title()is well-implemented and covers the expected functionality.R/outline-criteria.R (3)
Line range hint
18-32: The updated regex ino_is_roxygen_commentfunction correctly captures the intended patterns for roxygen comments.
75-75: The functiono_is_tab_plot_titleis correctly implemented to detect titles in tabulated data or plots.
Line range hint
124-192: Thedefine_outline_criteriafunction is robust, handling various file types and conditions effectively.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (5)
README.md (5)
Line range hint
22-22: Consider using a synonym for "hard" to enhance the expression in the sentence.
Line range hint
48-48: The phrase "recommended to avoid surprises" could be rephrased for clarity. Consider "recommended to prevent surprises".
Line range hint
52-52: The term "human-readable" should be hyphenated.
Line range hint
54-54: The phrase "in the long run" could be simplified to "eventually" for conciseness.
Line range hint
63-63: Clarify whether "to-do" (as a noun) or "to do" (as a verb) is intended here.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- R/outline-criteria.R (6 hunks)
- R/outline-roxy.R (1 hunks)
- README.md (12 hunks)
- man/complete_todo.Rd (2 hunks)
- man/link_gh_issue.Rd (1 hunks)
- man/markup_href.Rd (1 hunks)
- man/rename_files2.Rd (1 hunks)
- man/use_todo.Rd (1 hunks)
Files skipped from review due to trivial changes (4)
- man/link_gh_issue.Rd
- man/markup_href.Rd
- man/rename_files2.Rd
- man/use_todo.Rd
Files skipped from review as they are similar to previous changes (2)
- R/outline-roxy.R
- man/complete_todo.Rd
Additional Context Used
LanguageTool (5)
README.md (5)
Near line 22: To elevate your writing, try using a synonym here.
Context: ...tions across projects. Sometimes, it is hard to do that. reuseme also aims to help m...
Near line 48: The verb ‘recommended’ is used with the gerund form.
Context: ...on controlled with git (optional, but recommended to avoid surprises! No need to be hosted on repo...
Near line 52: This word is normally spelled with a hyphen.
Context: ...igned on Windows) - You use machine and human readable paths (i.e. no spaces, special charac...
Near line 54: ‘in the long run’ might be wordy. Consider a shorter alternative.
Context: ...your files, it can take away the pain in the long run! To take advantage of reuseme, it is h...
Near line 63: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...ns likeproj_switch(),proj_list(),use_todo()to be optimized. ## Example Since ...
Additional comments not posted (4)
R/outline-criteria.R (4)
Line range hint
18-32: Updated regex pattern ino_is_roxygen_commentto improve roxygen comment detection.Consider adding unit tests to ensure the new regex pattern correctly identifies all intended cases of roxygen comments.
75-75: Functiono_is_tab_plot_titlecorrectly identifies titles in tables or plots.
75-75: Functiono_is_section_titleeffectively identifies section titles.Consider adding more comprehensive unit tests to cover edge cases and ensure robustness.
Line range hint
124-193: Enhanced handling of roxygen comments indefine_outline_criteria.Consider improving the error message in line 140 to specify which files could not be parsed for better debugging.
Remove markup for tag = title. Add some tryCatch to facilitate error discovery.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (5)
README.md (5)
Line range hint
22-22: Consider using a different expression to enhance readability.The phrase "it is hard to do that" could be rephrased for clarity. Consider using "this can be challenging" to convey the difficulty more directly.
Line range hint
48-48: Use gerund form with "recommended".The phrase "recommended to avoid surprises" should use the gerund form for better grammatical accuracy: "recommended for avoiding surprises".
Line range hint
52-52: Add a hyphen to "human-readable".The term "human readable" should be hyphenated to "human-readable" when used as an adjective before a noun for better readability.
Line range hint
54-54: Consider simplifying "in the long run".The phrase "in the long run" could be simplified to "eventually" to make the sentence more concise.
Line range hint
63-63: Clarify the use of "to-do".The term "use_todo()" might be confusing. If it refers to a function for handling tasks, consider renaming it to "useToDo()" or explicitly mentioning that it manages "to-do items" in the documentation.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- R/outline-criteria.R (6 hunks)
- R/outline-roxy.R (1 hunks)
- R/outline.R (7 hunks)
- README.md (10 hunks)
- tests/testthat/_ref/test-roxygen-safeguard.R (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- R/outline-roxy.R
- R/outline.R
- tests/testthat/_ref/test-roxygen-safeguard.R
Additional Context Used
LanguageTool (5)
README.md (5)
Near line 22: To elevate your writing, try using a synonym here.
Context: ...tions across projects. Sometimes, it is hard to do that. reuseme also aims to help m...
Near line 48: The verb ‘recommended’ is used with the gerund form.
Context: ...on controlled with git (optional, but recommended to avoid surprises! No need to be hosted on repo...
Near line 52: This word is normally spelled with a hyphen.
Context: ...igned on Windows) - You use machine and human readable paths (i.e. no spaces, special charac...
Near line 54: ‘in the long run’ might be wordy. Consider a shorter alternative.
Context: ...your files, it can take away the pain in the long run! To take advantage of reuseme, it is h...
Near line 63: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...ns likeproj_switch(),proj_list(),use_todo()to be optimized. ## Example Since ...
Additional comments not posted (3)
R/outline-criteria.R (3)
Line range hint
18-32: Refined regex pattern for detecting roxygen comments.The updated regex pattern now correctly identifies lines that start with
#'followed by a space or are just#'. This change ensures more accurate detection of roxygen comments.
Line range hint
124-198: Significant enhancements indefine_outline_criteriato handle roxygen comments.The function now includes logic to check for the presence of roxygen comments and processes them accordingly. This is a crucial update for the integration of the roxygen2 parser as outlined in the PR objectives.
201-236: Introduceddefine_outline_criteria_roxyfor processing parsed roxygen data.This function processes the parsed roxygen data to integrate it into the outline criteria. It's a good separation of concerns, isolating roxygen-specific processing.
|
|
||
|
|
||
| define_outline_criteria_roxy <- function(x) { | ||
| # TODO merge with define_outline_criteria |
There was a problem hiding this comment.
Consider merging define_outline_criteria_roxy with define_outline_criteria.
The TODO comment suggests merging define_outline_criteria_roxy with define_outline_criteria to reduce redundancy. If you need assistance with this, I can help refactor the code or open a GitHub issue to track this task.
avoid parsing markdown tables, exclude some directories,
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (4)
README.md (4)
Line range hint
48-48: Consider using "recommended for" instead of "recommended to" for grammatical correctness.- recommended to avoid surprises! + recommended for avoiding surprises!
Line range hint
52-52: The term "human readable" should be hyphenated to "human-readable" when used as an adjective.- human readable paths + human-readable paths
Line range hint
54-54: Consider simplifying "in the long run" to "eventually" to make the sentence more concise.- it can take away the pain in the long run! + it can eventually take away the pain!
Line range hint
63-63: The term "to-do" should be hyphenated when used as a noun.- use_todo() + use_to-do()
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- .github/CONTRIBUTING.md (2 hunks)
- R/outline-criteria.R (4 hunks)
- R/outline.R (22 hunks)
- R/utils-proj.R (1 hunks)
- README.md (7 hunks)
- TODO.R (2 hunks)
- tests/testthat/_outline/roxy-general2.R (1 hunks)
- tests/testthat/test-outline-criteria.R (2 hunks)
Files skipped from review as they are similar to previous changes (4)
- R/utils-proj.R
- TODO.R
- tests/testthat/_outline/roxy-general2.R
- tests/testthat/test-outline-criteria.R
Additional context used
LanguageTool
.github/CONTRIBUTING.md
[style] ~12-~12: The word ‘Bigger’ tends to be overused. Consider an alternative.
Context: ...ding the comment in the first line. ## Bigger changes If you want to make a bigger change, i...
[uncategorized] ~50-~50: Loose punctuation mark.
Context: ... positive, *keep_outline_element(): if an element is missing from outli...
[uncategorized] ~52-~52: Possible missing comma found.
Context: ...e_important_element()` if an element is important [^1] [^1]: Importance definition is i...
[uncategorized] ~56-~56: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...tline element doesn't show as expected (i.e right position, but not enough or too m...
[style] ~60-~60: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... incorrect. *print.outline_report()if you want to improve styling ### Enha...
[grammar] ~100-~100: The usual collocation for “careful” is “with”, not “for”. Did you mean “careful with”?
Context: ...riteria tokeep_outline_element()Be careful for markdown vs non-markdown (i.e. section ...
[typographical] ~100-~100: Consider adding a comma here.
Context: ... markdown vs non-markdown (i.e. section title not the same) ```r # in our case we'd ...README.md
[style] ~22-~22: To elevate your writing, try using a synonym here.
Context: ...tions across projects. Sometimes, it is hard to do that. reuseme also aims to help m...
[grammar] ~48-~48: The verb ‘recommended’ is used with the gerund form.
Context: ...on controlled with git (optional, but recommended to avoid surprises! No need to be hosted on repo...
[misspelling] ~52-~52: This word is normally spelled with a hyphen.
Context: ...igned on Windows) - You use machine and human readable paths (i.e. no spaces, special charac...
[style] ~54-~54: ‘in the long run’ might be wordy. Consider a shorter alternative.
Context: ...your files, it can take away the pain in the long run! To take advantage of reuseme, it is h...
[grammar] ~63-~63: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...ns likeproj_switch(),proj_list(),use_todo()to be optimized. ## Example Since ...
Markdownlint
.github/CONTRIBUTING.md
8-8: Expected: 0 or 2; Actual: 1
Trailing spaces
9-9: Expected: 0 or 2; Actual: 1
Trailing spaces
14-14: Expected: 0 or 2; Actual: 1
Trailing spaces
15-15: Expected: 0 or 2; Actual: 1
Trailing spaces
23-23: Expected: 0 or 2; Actual: 1
Trailing spaces
24-24: Expected: 0 or 2; Actual: 1
Trailing spaces
35-35: Expected: 0 or 2; Actual: 1
Trailing spaces
40-40: Expected: 0 or 2; Actual: 1
Trailing spaces
47-47: Expected: 0 or 2; Actual: 1
Trailing spaces
52-52: Expected: 0 or 2; Actual: 1
Trailing spaces
56-56: Expected: 0 or 2; Actual: 1
Trailing spaces
49-49: Expected: 1; Actual: 2
Multiple consecutive blank lines
62-62: Expected: 1; Actual: 2
Multiple consecutive blank lines
63-63: Expected: 1; Actual: 3
Multiple consecutive blank lines
68-68: Expected: 1; Actual: 2
Multiple consecutive blank lines
71-71: Expected: 1; Actual: 2
Multiple consecutive blank lines
Additional comments not posted (12)
.github/CONTRIBUTING.md (1)
50-50: LGTM! The addition of specific debugging steps forproj_outline()enhances the clarity and utility of the contributing guidelines.R/outline-criteria.R (4)
21-32: LGTM! The updatedo_is_notebookfunction correctly implements notebook detection using file extensions and comment patterns.
35-50: LGTM! The enhancements too_is_roxygen_commentimprove its accuracy by considering theis_notebookparameter.
162-259: LGTM! The comprehensive updates todefine_outline_criteriaeffectively integrate new parameters and enhance the outlining process.
262-313: LGTM! The updates todefine_outline_criteria_roxycorrectly handle different content types and improve the handling of roxygen comments.R/outline.R (7)
16-16: Consider correcting the capitalization inconsistency in "Figures caption".
44-45: Clarify the permanence of theexclude_testsparameter to ensure API stability.
56-56: Address the deprecation ofprint_todomore clearly in the documentation.
546-546: Add error handling around the call tofind_pkg_org_repo.
104-111: Refactor deprecated parameter handling for clarity and maintainability.
176-176: Ensure that thedefine_outline_criteriafunction call includes all necessary parameters.Verification successful
The search results indicate that the
define_outline_criteriafunction is defined with three parameters:.data,exclude_todos, anddir_common. The instance inR/outline.Rincludesfile_content,exclude_todos, anddir_common, which appears correct. However, to be thorough, we should verify all instances ofdefine_outline_criteriato ensure they include the necessary parameters.
The function
define_outline_criteriais defined with three parameters:.data,exclude_todos, anddir_common. The search results show that the instance inR/outline.Rincludesfile_content,exclude_todos, anddir_common, which aligns with the function definition.All instances of
define_outline_criteriain the codebase include the necessary parameters.
R/outline.R: file_sections00 <- define_outline_criteria(file_content, exclude_todos = exclude_todos, dir_common)This confirms that the function call includes all necessary parameters.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all calls to `define_outline_criteria` include the necessary parameters. # Test: Search for the function usage. Expect: All calls include the necessary parameters. rg --type r $'define_outline_criteria'Length of output: 627
Script:
#!/bin/bash # Verify the function definition and all instances of `define_outline_criteria` to ensure they include the necessary parameters. # Search for the function definition to confirm parameters rg --type r --context 5 $'define_outline_criteria <- function' # Search for all instances of `define_outline_criteria` to ensure they include the necessary parameters rg --type r $'define_outline_criteria'Length of output: 1313
90-92: Theprint_todoparameter is deprecated. Ensure that the deprecation is handled consistently across the codebase.Verification successful
The
print_todoparameter is marked as deprecated and has several occurrences in theR/outline.Rfile. Here are the specific lines and their context:
- Line 1: Documentation comment marking it as deprecated.
- Line 2: Function parameter definition.
- Line 3: Conditional check using
lifecycle::is_present.- Line 4: Assignment to
exclude_todos.- Line 5: Usage in a string for
whatparameter.These occurrences suggest that the deprecation is being handled correctly by:
- Marking the parameter as deprecated in the documentation.
- Checking if the deprecated parameter is present.
- Assigning its value to a new parameter (
exclude_todos).- Using it in a string for logging or debugging purposes.
Given this context, it appears that the deprecation is managed properly. However, to ensure thorough verification, we should check if there are any other occurrences of
print_todooutside of this file that might not be handled correctly.
The search for the
print_todoparameter outside ofR/outline.Rdid not produce any results, indicating that the deprecated parameter is not used elsewhere in the codebase. This suggests that the deprecation is handled consistently and appropriately.
- The
print_todoparameter is marked as deprecated in the documentation.- It is checked for presence using
lifecycle::is_present.- Its value is reassigned to a new parameter (
exclude_todos).- It is used in a string for logging or debugging purposes.
These steps confirm that the deprecation is managed correctly within
R/outline.R, and there are no inappropriate usages elsewhere in the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the deprecated `print_todo` parameter is handled consistently across the codebase. # Test: Search for the usage of `print_todo`. Expect: No occurrences outside of deprecation handling. rg --type r $'print_todo'Length of output: 341
Script:
#!/bin/bash # Description: Verify that the deprecated `print_todo` parameter is not used inappropriately outside of R/outline.R. # Test: Search for the usage of `print_todo` in the entire codebase. Expect: No occurrences outside of deprecation handling. rg --type r $'print_todo' --glob '!R/outline.R'Length of output: 46
…le to is_doc_title. Remove code blocks from outline in roxy cpmmms
There was a problem hiding this comment.
Actionable comments posted: 8
Outside diff range and nitpick comments (6)
tests/testthat/_outline/quarto-caps.md (2)
13-13: Consider adding a comma after "html" for better readability.Tools
LanguageTool
[uncategorized] ~13-~13: Possible missing comma found.
Context: ...-solid fa-tags"> Remove the html and if possible, just print an emoji? maybe...
24-24: Suggestion to remove the backslash for cleaner markdown syntax.tests/testthat/_snaps/outline.md (1)
Line range hint
115-115: Reduce consecutive blank lines to improve readability and adhere to standard formatting practices.Tools
Markdownlint
40-40: null
Multiple top-level headings in the same documentREADME.md (3)
19-27: Consider revising the repetitive structure for clarity.The paragraph from lines 19 to 27 repeats the phrase "Sometimes" multiple times, which might affect the readability. Consider restructuring these sentences to flow more naturally.
Tools
LanguageTool
[style] ~26-~26: To elevate your writing, try using a synonym here.
Context: ...tions across projects. Sometimes, it is hard to do that. reuseme also aims to help m...
Line range hint
83-83: Add missing space between words.- In interactive sessions, you can use the `screenshot()` function to access an image in the clipboard and save it as .png in a `figures/` or `images/`directory in your RStudio project. + In interactive sessions, you can use the `screenshot()` function to access an image in the clipboard and save it as .png in a `figures/` or `images/` directory in your RStudio project.Tools
LanguageTool
[style] ~26-~26: To elevate your writing, try using a synonym here.
Context: ...tions across projects. Sometimes, it is hard to do that. reuseme also aims to help m...
Line range hint
107-148: Review the table structure for accessibility.The table structure from lines 107 to 148 is complex and might not be fully accessible. Consider simplifying the structure or providing alternative text descriptions for better accessibility.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (13)
- R/outline-criteria.R (4 hunks)
- R/outline-roxy.R (1 hunks)
- R/outline.R (22 hunks)
- R/rename.R (4 hunks)
- R/todo.R (5 hunks)
- README.Rmd (4 hunks)
- README.md (11 hunks)
- TODO.R (2 hunks)
- man/outline.Rd (7 hunks)
- tests/testthat/_outline/my-analysis.R (2 hunks)
- tests/testthat/_outline/quarto-caps.md (1 hunks)
- tests/testthat/_outline/roxy-general2.R (1 hunks)
- tests/testthat/_snaps/outline.md (2 hunks)
Files skipped from review as they are similar to previous changes (7)
- R/outline-roxy.R
- R/rename.R
- R/todo.R
- README.Rmd
- TODO.R
- tests/testthat/_outline/my-analysis.R
- tests/testthat/_outline/roxy-general2.R
Additional context used
LanguageTool
tests/testthat/_outline/quarto-caps.md
[uncategorized] ~13-~13: Possible missing comma found.
Context: ...-solid fa-tags"> Remove the html and if possible, just print an emoji? maybe...README.md
[style] ~26-~26: To elevate your writing, try using a synonym here.
Context: ...tions across projects. Sometimes, it is hard to do that. reuseme also aims to help m...
[grammar] ~64-~64: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...ns likeproj_switch(),proj_list(),reuseme::use_todo()to be optimized. ## Example Since ...
[uncategorized] ~83-~83: A punctuation mark might be missing here.
Context: ...d and save it as .png in afigures/orimages/directory in your RStudio proje...
Markdownlint
tests/testthat/_snaps/outline.md
115-115: Expected: 1; Actual: 2
Multiple consecutive blank lines
40-40: null
Multiple top-level headings in the same document
51-51: null
Multiple top-level headings in the same document
64-64: null
Multiple top-level headings in the same document
90-90: null
Multiple top-level headings in the same document
105-105: null
Multiple top-level headings in the same document
Additional comments not posted (4)
man/outline.Rd (2)
25-26: Confirm that the newexclude_testsparameter is consistently used across the project.Also applies to: 36-37
Verification successful
The
exclude_testsparameter is consistently used across the project in the following locations:
NEWS.mdman/outline.RdR/outline.RThis confirms that the parameter is integrated and documented appropriately.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `exclude_tests` parameter. # Test: Search for the usage of `exclude_tests`. Expect: Consistent and correct usage. rg --type R $'exclude_tests' man/outline.RdLength of output: 68
Script:
#!/bin/bash # Description: Verify the usage of the `exclude_tests` parameter. # Test: Search for the usage of `exclude_tests` across the entire codebase. Expect: Consistent and correct usage. rg 'exclude_tests'Length of output: 946
16-18: Ensure that the deprecatedprint_todoparameter is handled correctly in all function calls.Verification successful
The deprecated
print_todoparameter is correctly handled in the codebase. It is marked as deprecated and replaced byexclude_todos, with appropriate warnings issued whenprint_todois used.
man/outline.Rd: The parameter is documented as deprecated.R/outline.R: The parameter is checked for presence, and a deprecation warning is issued, advising the use ofexclude_todosinstead.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of the deprecated `print_todo` parameter. # Test: Search for function calls using `print_todo`. Expect: Proper handling or removal. rg --type R $'print_todo' man/outline.RdLength of output: 65
Script:
#!/bin/bash # Description: Verify the handling of the deprecated `print_todo` parameter. # Test: Search for function calls using `print_todo`. Expect: Proper handling or removal. rg 'print_todo'Length of output: 613
Script:
#!/bin/bash # Description: Verify the handling of the deprecated `print_todo` parameter in R/outline.R. # Test: Extract lines around the usage of `print_todo` to ensure proper handling or replacement. rg -A 5 -B 5 'print_todo' R/outline.RLength of output: 1404
README.md (1)
51-55: Ensure correct usage of hyphen in "to-do".- You use machine and human-readable paths (i.e. no spaces, special characters) (Tip: don’t hesitate to rename your files (`reuseme::rename_files2()`), your future self will thank you! + You use machine and human-readable paths (i.e. no spaces, special characters) (Tip: don’t hesitate to rename your files (`reuseme::rename_files2()`), your future self will thank you for managing your to-do list effectively!Likely invalid or redundant comment.
R/outline.R (1)
16-16: Correct capitalization inconsistency in "Figures caption".- #' * Figures caption in Quarto documents + #' * Figures caption in Quarto documentsLikely invalid or redundant comment.
R/outline.R
Outdated
| @@ -197,7 +208,7 @@ file_outline <- function(pattern = NULL, | |||
| } | |||
| # File outline =================== | |||
| # strip outline element .data$outline = `# Section 1` becomes `Section 1` | |||
| file_sections1 <- display_outline_element(file_sections0) | |||
| file_sections1 <- display_outline_element(file_sections0, dir_common) | |||
There was a problem hiding this comment.
Optimize the display_outline_element function to improve performance and maintainability.
display_outline_element <- function(.data, dir_common) {
# Simplified version of the function
x <- .data
org_repo <- find_pkg_org_repo(dir_common, unique(x$file))
if (!is.null(org_repo)) {
x$outline_el <- link_local_gh_issue(x$content, org_repo)
} else {
x$outline_el <- x$content
}
x$outline_el <- purrr::map_chr(x$outline_el, \(x) link_gh_issue(x, org_repo)) # to add link to GitHub.
x$outline_el <- purrr::map_chr(x$outline_el, markup_href)
if (any(x$is_obj_caption)) {
x$outline_el[x$is_obj_caption] <- extract_object_captions(x$file[x$is_obj_caption])
}
x <- dplyr::mutate(
x,
outline_el = dplyr::case_when(
is_todo_fixme ~ stringr::str_extract(outline_el, "(TODO.+)|(FIXME.+)|(WORK.+)"),
is_test_name ~ stringr::str_extract(outline_el, "test_that\\(['\"](.+)['\"],\\s?\\{", group = 1),
is_cli_info ~ stringr::str_extract(outline_el, "[\"'](.{5,})[\"']") |> stringr::str_remove_all("\""),
# Add related topic if available
tag == "title" & !is_n...| #' print a less verbose output with sections. | ||
| #' @param alpha Whether to show in alphabetical order | ||
| #' @param dir_tree If `TRUE`, will print the [fs::dir_tree()] or non-R files in | ||
| #' the directory | ||
| #' @param recent_only Show outline for recent files | ||
| #' @param dir_common (Do not use it) | ||
| #' @param print_todo `r lifecycle::badge("deprecated")`. Use `exclude_todos` instead. |
There was a problem hiding this comment.
Address the deprecation of print_todo more clearly in the documentation.
The parameter print_todo is marked as deprecated, but the documentation could better highlight what users should use instead. Consider enhancing the documentation to guide users more effectively.
| #' The print method will show the document title for context. Previously `regex_outline`. | ||
| #' @param exclude_tests Should tests be displayed? (Not sure if this argument will stay, Will have to think) |
There was a problem hiding this comment.
Clarify the permanence of the exclude_tests parameter to ensure API stability.
The comment indicates uncertainty about whether the exclude_tests parameter will remain. It's important to resolve this uncertainty before the release to avoid future breaking changes.
| if (lifecycle::is_present(print_todo)) { | ||
| exclude_todos <- !print_todo | ||
| lifecycle::deprecate_warn( | ||
| when = "0.0.3", | ||
| what = "file_outline(print_todo)", | ||
| with = "file_outline(exclude_todos)" | ||
| ) | ||
| } |
There was a problem hiding this comment.
Refactor deprecated parameter handling for clarity and maintainability.
The handling of the deprecated print_todo parameter could be clearer. Consider refactoring this block to separate the deprecation logic from the main function logic, improving readability.
if (lifecycle::is_present(print_todo)) {
exclude_todos <- !print_todo
deprecate_print_todo()
}
deprecate_print_todo <- function() {
lifecycle::deprecate_warn(
when = "0.0.3",
what = "file_outline(print_todo)",
with = "file_outline(exclude_todos)"
)
}| `i` A long ggplot2 title with more details2 | ||
| `i` A long ggplot2 title with more details3. |
There was a problem hiding this comment.
Tip
Codebase Verification
The ggplot2 titles in the file tests/testthat/_snaps/outline.md show slight variations in their formatting. Ensure that the titles follow a consistent format unless there is a specific reason for the differences.
- Line 14:
A long ggplot2 title with more details2 - Line 15:
A long ggplot2 title with more details3.
Analysis chain
Ensure consistency in the format of the ggplot2 titles.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the consistency of ggplot2 titles across the file.
# Test: Search for ggplot2 titles. Expect: Consistent formatting.
rg --type markdown $'ggplot2 title' tests/testthat/_snaps/outline.md
Length of output: 303
R/outline-criteria.R
Outdated
| o_is_notebook <- function(x, file, file_ext, line) { | ||
| # Like roxy comments and first line = --, 2nd title. | ||
| # x$is_notebook <- grepl("notebook.*\\.R", x$file) | ||
| # Detect #' --- | ||
| any_notebooks <- grep("^#' ---", x[line == 1 & file_ext == "R"], fixed = FALSE) | ||
| if (length(any_notebooks) > 0L) { | ||
| is_notebook <- file %in% file[line == 1 & file_ext == "R"][any_notebooks] | ||
| } else { | ||
| is_notebook <- FALSE | ||
| } | ||
| is_notebook | ||
| } |
There was a problem hiding this comment.
Refactor o_is_notebook for clarity.
- any_notebooks <- grep("^#' ---", x[line == 1 & file_ext == "R"], fixed = FALSE)
+ any_notebooks <- grep("^#' ---", x[line == 1 & tolower(file_ext) == "r"], ignore.case = TRUE)Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| o_is_notebook <- function(x, file, file_ext, line) { | |
| # Like roxy comments and first line = --, 2nd title. | |
| # x$is_notebook <- grepl("notebook.*\\.R", x$file) | |
| # Detect #' --- | |
| any_notebooks <- grep("^#' ---", x[line == 1 & file_ext == "R"], fixed = FALSE) | |
| if (length(any_notebooks) > 0L) { | |
| is_notebook <- file %in% file[line == 1 & file_ext == "R"][any_notebooks] | |
| } else { | |
| is_notebook <- FALSE | |
| } | |
| is_notebook | |
| } | |
| o_is_notebook <- function(x, file, file_ext, line) { | |
| # Like roxy comments and first line = --, 2nd title. | |
| # x$is_notebook <- grepl("notebook.*\\.R", x$file) | |
| # Detect #' --- | |
| any_notebooks <- grep("^#' ---", x[line == 1 & tolower(file_ext) == "r"], ignore.case = TRUE) | |
| if (length(any_notebooks) > 0L) { | |
| is_notebook <- file %in% file[line == 1 & file_ext == "R"][any_notebooks] | |
| } else { | |
| is_notebook <- FALSE | |
| } | |
| is_notebook | |
| } |
| o_is_roxygen_comment <- function(x, file_ext = NULL, is_notebook = FALSE) { | ||
| if (!is.null(file_ext)) { | ||
| is_r_file <- tolower(file_ext) == "r" | ||
| is_r_file <- tolower(file_ext) == "r" & !is_notebook | ||
| } else { | ||
| is_r_file <- TRUE | ||
| is_r_file <- !is_notebook | ||
| } | ||
|
|
||
| if (!any(is_r_file)) { | ||
| return(FALSE) | ||
| } | ||
|
|
||
| ifelse(rep(is_r_file, length.out = length(x)), stringr::str_starts(x, "#'\\s"), FALSE) | ||
| ifelse( | ||
| rep(is_r_file, length.out = length(x)), | ||
| grepl("^#'\\s|^#'$", x), # detect roxygen comments in R files | ||
| FALSE # not a roxy comment in Rmd files, fusen is an exception? | ||
| ) |
There was a problem hiding this comment.
Improve readability in o_is_roxygen_comment.
- if (!is.null(file_ext)) {
- is_r_file <- tolower(file_ext) == "r" & !is_notebook
- } else {
- is_r_file <- !is_notebook
- }
+ is_r_file <- (!is.null(file_ext) && tolower(file_file_ext) == "r" || is.null(file_ext)) && !is_notebookCommittable suggestion was skipped due low confidence.
|
@coderabbitai pause |
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (3)
README.md (1)
Line range hint
51-65: Consider revising the phrase for clarity.- recommended for avoiding surprises! No need to be hosted on + recommended to avoid surprises! There's no need to be hosted onThis change improves the readability and grammatical correctness of the sentence.
R/outline.R (2)
[!TIP]
Codebase VerificationThe parameters
exclude_testsandexclude_todosare documented inR/outline.R. However, there are no test cases found for these parameters in thetests/directory.
- Documentation for
exclude_testsandexclude_todosis present inR/outline.R.- No test cases for
exclude_testsandexclude_todoswere found in thetests/directory.Please add test cases for the new parameters
exclude_testsandexclude_todosto ensure they are properly tested.
Analysis chain
Line range hint
308-351: Ensure that the new parametersexclude_testsandexclude_todosindir_outlinefunction are properly documented and tested.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the documentation and tests for new parameters in `dir_outline`. # Test: Search for documentation mentions. Expect: At least one mention per parameter. rg --type r $'exclude_tests|exclude_todos' R/ # Test: Search for test cases. Expect: At least one test case per parameter. rg --type r $'exclude_tests|exclude_todos' tests/Length of output: 1821
[!TIP]
Codebase VerificationThe new parameters
exclude_testsandexclude_todosin theproj_outlinefunction are documented in theR/outline.Rfile. However, there are no test cases for these parameters in thetests/directory.
- Ensure that test cases for
exclude_testsandexclude_todosare added to verify their functionality.Analysis chain
Line range hint
243-259: Ensure that the new parametersexclude_testsandexclude_todosinproj_outlinefunction are properly documented and tested.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the documentation and tests for new parameters in `proj_outline`. # Test: Search for documentation mentions. Expect: At least one mention per parameter. rg --type r $'exclude_tests|exclude_todos' R/ # Test: Search for test cases. Expect: At least one test case per parameter. rg --type r $'exclude_tests|exclude_todos' tests/Length of output: 1821
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- R/outline.R (22 hunks)
- README.md (11 hunks)
- playground/outline-tree.R (1 hunks)
- tests/testthat/_outline/tree.qmd (1 hunks)
Files skipped from review due to trivial changes (1)
- tests/testthat/_outline/tree.qmd
Additional context used
LanguageTool
README.md
[style] ~26-~26: To elevate your writing, try using a synonym here.
Context: ...tions across projects. Sometimes, it is hard to do that. reuseme also aims to help m...
[grammar] ~64-~64: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...ns likeproj_switch(),proj_list(),reuseme::use_todo()to be optimized. ## Example Since ...
Additional comments not posted (7)
README.md (2)
Line range hint
19-55: Clear and informative installation instructions.The installation section is well-written and provides clear instructions on how to install the package from different sources.
Tools
LanguageTool
[style] ~26-~26: To elevate your writing, try using a synonym here.
Context: ...tions across projects. Sometimes, it is hard to do that. reuseme also aims to help m...
107-136: Effective use of comparison to highlight package features.The comparison table effectively highlights the differences and advantages of using
reusemeoverusethis, making it easier for users to understand the benefits.R/outline.R (5)
44-45: Clarify the permanence of theexclude_testsparameter to ensure API stability.
56-56: Address the deprecation ofprint_todomore clearly in the documentation.
104-111: Refactor deprecated parameter handling for clarity and maintainability.
211-211: Optimize thedisplay_outline_elementfunction to improve performance and maintainability.
16-16: Correct the capitalization in "Figures caption".- #' * Figures caption in Quarto documents + #' * Figures Caption in Quarto documentsLikely invalid or redundant comment.
| # filter(sum(value) > 1, .by = c(file_short, title, line_id)) | ||
| dplyr::filter(value) %>% | ||
| # We drop these because they don't serve to add much context to TODOs (they don't affect heirarchy) | ||
| dplyr::filter(type != "tab_or_plot_title") %>% | ||
|
|
||
| # Some useful definitions! | ||
| dplyr::mutate( | ||
| title = dplyr::coalesce(outline_el, title_el), | ||
| file_short = fs::path_file(file), | ||
| n_leading_hash = type %>% dplyr::case_match( | ||
| c("todo_fixme", "tab_or_plot_title") ~ NA, | ||
| .default = n_leading_hash | ||
| ) | ||
| ) %>% | ||
|
|
||
| # For each file, stick a item at the top of the outline | ||
| dplyr::group_by(file, file_short) %>% | ||
| group_modify(\(data, group) data %>% add_row( | ||
| .before = 0, | ||
| n_leading_hash = -1, | ||
| title = group$file_short, | ||
| type = "file" | ||
| )) %>% | ||
|
|
||
| mutate( | ||
| # Processing how title displays based on type | ||
| print_title = dplyr::case_match( | ||
| type, | ||
| "todo_fixme" ~ link, | ||
| .default = link_rs_api | ||
| ) %>% dplyr::coalesce(title) %>% purrr::map_chr(cli::format_inline), | ||
|
|
||
| # Assign TODO items (and other items missing n_leading_hash) | ||
| # to be indented under the last seen header level | ||
| indent = dplyr::coalesce(n_leading_hash, zoo::na.locf0(n_leading_hash+1)), | ||
|
|
||
| # If there are any headers that skip an intermediate level, pick them up | ||
| skip_level = indent > dplyr::lag(indent)+1, | ||
| skip_level_should_be = ifelse(skip_level, dplyr::lag(indent)+1, NA), | ||
| skip_level_adjust = dplyr::case_when( | ||
| # All the items below on the outline should be adjusted backwards | ||
| skip_level ~ skip_level_should_be-indent, | ||
| # Unless we reach a point on the outline where we're back up in | ||
| # the hierachy, so stop adjusting. | ||
| indent <= zoo::na.locf0(skip_level_should_be) ~ 0 | ||
| ) %>% | ||
| # Carry the adjustments to later rows | ||
| zoo::na.locf0() %>% dplyr::coalesce(0), | ||
|
|
||
| indent = indent + skip_level_adjust | ||
| ) |> | ||
| dplyr::ungroup() |> | ||
| dplyr::select(title, type, n_leading_hash, indent, print_title) | ||
|
|
||
| dat_ready_to_tree <- outline_data %>% | ||
| dplyr::mutate( | ||
| # Give items IDs so titles do not have to be unique | ||
| item_id = as.character(dplyr::row_number()), | ||
| indent_wider = indent, | ||
| x = TRUE | ||
| ) %>% | ||
|
|
||
| # We need these wide cumsum `header1` type fields to determine which items belong to which parents | ||
| tidyr::pivot_wider(names_from = indent_wider, values_from = x, values_fill = FALSE, names_prefix = "header") %>% | ||
| dplyr::mutate(dplyr::across(dplyr::starts_with("header"), cumsum)) %>% | ||
|
|
||
| # For each row, pick the IDs of all direct children from the outline | ||
| purrr::pmap(function(...) with(list(..., childdata = .), tibble( | ||
| title, | ||
| print_title, | ||
| indent, | ||
| item_id, | ||
| type, | ||
| parent_level_id = get(stringr::str_c("header", indent)), | ||
| children_ids = childdata %>% | ||
| dplyr::rename(childindent = indent) %>% | ||
| dplyr::filter( | ||
| childindent == indent+1, | ||
| cumsum(childindent == indent) == parent_level_id | ||
| ) %>% | ||
| dplyr::pull(item_id) %>% list() | ||
| ))) %>% | ||
| purrr::list_rbind() %>% | ||
| # View() | ||
| dplyr::select(item_id, children_ids, print_title) | ||
|
|
||
| dat_ready_to_tree |> | ||
| cli::tree() | ||
|
|
||
| # browse-pkg | ||
| dat_ready_to_tree |> | ||
| cli::tree("5") | ||
|
|
||
| dat_ready_to_tree |> | ||
| cli::tree("10") | ||
| dat_ready_to_tree |> | ||
| dplyr::filter(purrr::map_lgl(children_ids, \(x) length(x) > 0)) | ||
|
|
||
| dat_ready_to_tree |> | ||
| cli::tree("13") |
There was a problem hiding this comment.
Optimize data processing and ensure exclusive operations.
The script performs multiple data transformations using dplyr and tidyr. Consider optimizing these operations to improve performance, especially since the PR mentions a significant slowdown. Additionally, ensure that the operations like filtering and grouping are mutually exclusive to avoid data inconsistencies.
R/outline.R
Outdated
| in_active_project <- suppressMessages( | ||
| tryCatch( |
There was a problem hiding this comment.
Add error handling around the call to define_outline_criteria to ensure robustness in case of exceptions.
tryCatch({
file_sections00 <- define_outline_criteria(file_content, exclude_todos = exclude_todos, dir_common)
}, error = function(e) {
cli::cli_abort("Failed to define outline criteria: ", e$message)
})Also applies to: 176-176
| exclude_todos = FALSE, | ||
| recent_only = FALSE, | ||
| print_todo = deprecated()) { |
There was a problem hiding this comment.
Tip
Codebase Verification
The search results indicate that there are no test cases for the new parameters exclude_todos, recent_only, and print_todo in the tests/ directory. This confirms that the parameters are documented but not tested.
- Ensure that the new parameters
exclude_todos,recent_only, andprint_todoare properly tested.
Analysis chain
Ensure that the new parameters exclude_todos, recent_only, and print_todo are properly documented and tested.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the documentation and tests for new parameters in `file_outline`.
# Test: Search for documentation mentions. Expect: At least one mention per parameter.
rg --type r $'exclude_todos|recent_only|print_todo' R/
# Test: Search for test cases. Expect: At least one test case per parameter.
rg --type r $'exclude_todos|recent_only|print_todo' tests/
Length of output: 2145
Script:
#!/bin/bash
# Description: Verify the presence of test cases for new parameters in `file_outline`.
# Test: Search for test cases mentioning the new parameters. Expect: At least one test case per parameter.
rg --type r $'exclude_todos|recent_only|print_todo' tests/
Length of output: 59
| extract_object_captions <- function(file) { | ||
| rlang::check_installed("lightparser", "to parse qmd files add chunk caption to outline.") | ||
| # we want fig-cap, tbl-cap and title | ||
| caps <- NULL | ||
| unique_file <- unique(file) | ||
| # ThinkR-open/lightparser#8 | ||
| for (i in seq_along(unique_file)) { | ||
| # FIXME find a way to be as consistent as lightparser, but faster. | ||
| dat <- tryCatch( | ||
| lightparser::split_to_tbl(unique_file[i]), | ||
| error = function(e) { | ||
| # workaround ThinkR-open/lightparser#11 | ||
| tmp <- withr::local_tempfile( | ||
| lines = c( | ||
| "---", | ||
| "title: dummy", | ||
| "---", | ||
| readLines(unique_file[i], warn = FALSE) | ||
| ) | ||
| ) | ||
| lightparser::split_to_tbl(tmp) | ||
| } | ||
| ) | ||
| dat <- dplyr::filter(dat, type == "block")$params | ||
| # Remove NA.. | ||
| dat <- purrr::discard(dat,\(x) isTRUE(is.na(x))) | ||
| # tidyverse/purrr#1081 | ||
| if (length(dat) > 0) { | ||
| # We use `format()` in case a variable is used to name the caption. | ||
| tryCatch(caps <- c(caps, dat |> purrr::map_chr(\(x) format(x[["fig-cap"]] %||% x[["tbl-cap"]] %||% x[["title"]] %||% x[["fig.cap"]] %||% x[["tbl.cap"]] %||% x[["tab.cap"]] %||% x[["cap"]] %||% "USELESS THING"))), error = function(e) { | ||
| cli::cli_abort("Error in {.file {unique_file[i]}}", parent = e) | ||
| }) | ||
| } | ||
| } | ||
| # used as a default to make sure purrr doesn't complain | ||
| caps <- caps[caps != "USELESS THING"] | ||
| if (length(caps) != length(file)) { | ||
| cli::cli_abort("error! :(, caps = {length(caps)}, file = {length(file)} in file {.file {unique_file}}") | ||
| } | ||
| caps |> stringr::str_squish() | ||
| } |
There was a problem hiding this comment.
Review the performance implications of the extract_object_captions function, especially the use of lightparser.
Consider optimizing the function to reduce the performance overhead, possibly by caching results or reducing the frequency of calls to lightparser.
Actions performedReviews paused. |
Lint + add leading . as acceptable R variable name. Add home repo support to link_gh_issue Add active_rs_doc_nav() Add support for renaming md files Improve `o_is_test_name()` to exclude empty tests Improve `o_is_todo_fixme()` to include book. Rename `o_is_object_title()` to `o_is_tab_plot_title() Rename test files Remove some variables from outline...
Merge commit 'b4803a244a484a7b686d05f28ea7be91f4d412c3' #Conflicts: # NEWS.md # R/outline-criteria.R # R/outline.R # R/screenshot.R # R/todo.R # README.Rmd # README.md # TODO.R # tests/testthat/_outline/my-analysis.md # tests/testthat/_snaps/outline-criteria.md # tests/testthat/_snaps/outline.md # tests/testthat/test-outline.R
Merge commit '6ab0c3fa1b4df2c73d9d1f20b606561a6c519fc6' #Conflicts: # R/outline.R
Almost ready? tested on many projects.
Make sure elements are as exclusive as possible..
Slows things down significantly..