Expose internals used with other style guides #1043

IndrajeetPatil · 2022-10-28T14:27:22Z

Re-implementation of abandoned #1037, which had complex merge conflicts.

Closes #974.

Progress tracker

Export compute_parse_data_nested() helper
Export is_*() helpers
Add examples to new exports
Check function and argument names are okay (since these are now user-facing)
Code clean-up
~~Update NEWS~~ ({fledge} is supposed to take care of this)
Code review

codecov-commenter · 2022-10-28T14:41:07Z

Codecov Report

Merging #1043 (24684f6) into main (4c1cb51) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1043      +/-   ##
==========================================
- Coverage   91.06%   91.05%   -0.02%     
==========================================
  Files          46       46              
  Lines        2687     2682       -5     
==========================================
- Hits         2447     2442       -5     
  Misses        240      240

Impacted Files	Coverage Δ
R/detect-alignment.R	`97.80% <ø> (ø)`
R/initialize.R	`96.96% <ø> (ø)`
R/nest.R	`100.00% <ø> (ø)`
R/nested-to-tree.R	`92.85% <ø> (ø)`
R/style-guides.R	`99.43% <ø> (ø)`
R/utils-navigate-nest.R	`81.48% <ø> (ø)`
R/expr-is.R	`86.11% <100.00%> (ø)`
R/rules-indention.R	`100.00% <100.00%> (ø)`
R/rules-line-breaks.R	`100.00% <100.00%> (ø)`
R/rules-spaces.R	`98.29% <100.00%> (-0.05%)`	⬇️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

github-actions · 2022-10-28T15:05:29Z

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 89433fa is merged into main:

:rocket:cache_applying: 40.7ms -> 36ms [-15.28%, -8.05%]
:ballot_box_with_check:cache_recording: 1.19s -> 1.19s [-1.76%, +1.46%]
:ballot_box_with_check:without_cache: 2.99s -> 2.98s [-1.65%, +0.81%]

Further explanation regarding interpretation and methodology can be found in the documentation.

Robinlovelace · 2022-10-30T16:48:32Z

Hi @IndrajeetPatil just following-up on this, is there any way I can help progress? Will be great to be able to have styler.* packages building on this!

IndrajeetPatil · 2022-10-30T18:11:42Z

R/expr-is.R

+#' pd <- compute_parse_data_nested(code)
+#' is_curly_expr(pd)
+#' is_curly_expr(pd$child$`17`$child$`14`)
+#'


The blank lines are deliberate.

Without them, all examples are squished together like a centipede, and it's hard to tell where one ends and the other starts.

IndrajeetPatil · 2022-10-30T18:12:15Z

R/expr-is.R

+#' is_function_declaration(pd$child$`12`$child$`11`)
+#'
+#' @export
+is_function_declaration <- function(pd) {


I think this is a better name than is_function_dec().

IndrajeetPatil · 2022-10-30T18:12:51Z

R/expr-is.R

+#' is_conditional_expr(pd$child$`24`)
+#'
+#' @export
+is_conditional_expr <- function(pd) {


I think this is a better name than is_cond_expr().

But why do we need to change all the *_expr() functions? Their current names are expressive as they are.

no sorry I read your comment wrongly (which is why I deleted it). I thought you wanted *_expression() everywhere, but that is (obviously) not the case.

I see.

Okay, then this should be ready for your review. I don't have any additional changes in mind.

IndrajeetPatil · 2022-10-30T18:20:19Z

Hi @Robinlovelace, I am done on my end. Now I await code review. Once that's done, I think this should be good to go and might even be part of the next release (cf. #1044).

github-actions · 2022-10-30T18:23:12Z

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 11b3264 is merged into main:

:rocket:cache_applying: 28.7ms -> 25.7ms [-11.47%, -9.73%]
:ballot_box_with_check:cache_recording: 778ms -> 777ms [-0.56%, +0.26%]
:ballot_box_with_check:without_cache: 1.95s -> 1.95s [-0.27%, +0.41%]

Further explanation regarding interpretation and methodology can be found in the documentation.

Robinlovelace · 2022-10-30T19:15:37Z

That is a great update, thanks. 🤞 for the next release!

github-actions · 2022-11-02T19:28:14Z

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 11b3264 is merged into main:

:ballot_box_with_check:cache_applying: 30.9ms -> 30.6ms [-2.07%, +0.31%]
:ballot_box_with_check:cache_recording: 955ms -> 951ms [-2.12%, +1.35%]
:ballot_box_with_check:without_cache: 2.4s -> 2.42s [-0.73%, +2.71%]

Further explanation regarding interpretation and methodology can be found in the documentation.

lorenzwalthert · 2022-11-02T20:52:32Z

Thanks for the work on this PR. @IndrajeetPatil I saw the review request. Please do not re-request a review from me. I don't forget about them, but I don't have time to look into it now but I will so in due time.

this should be more robust against changes in the R parser with the goal to avoid CRAN submission due to failing examples

lorenzwalthert · 2022-11-07T16:18:17Z

@IndrajeetPatil can you have a look at my last commit (can't request a review from you for some reason)? After this one is approved by you, I will prepare the release as discussed in #1044.

IndrajeetPatil · 2022-11-07T16:25:23Z

can't request a review from you for some reason

The one who made PR can't review it themselves. That's why you can't assign me to review my own PR.

can you have a look at my last commit

Your changes look good to me. Thanks.

github-actions · 2022-11-07T16:26:43Z

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 290a73a is merged into main:

:ballot_box_with_check:cache_applying: 23.7ms -> 23.7ms [-0.44%, +0.27%]
:ballot_box_with_check:cache_recording: 745ms -> 744ms [-0.72%, +0.37%]
:ballot_box_with_check:without_cache: 1.86s -> 1.86s [-0.65%, +0.19%]

Further explanation regarding interpretation and methodology can be found in the documentation.

Follow-up on #1043

Expose internals used with other style guides

59958a1

IndrajeetPatil mentioned this pull request Oct 28, 2022

Expose internals used with other style guides #1037

Closed

6 tasks

github-actions bot and others added 2 commits October 28, 2022 14:30

pre-commit

880a145

minor

fce1680

IndrajeetPatil added 3 commits October 30, 2022 19:02

Add examples for remaining new exports

5bfd6db

Example for compute_parse_data_nested()

8544a3c

Update compute_parse_data_nested.Rd

d5d2546

IndrajeetPatil commented Oct 30, 2022

View reviewed changes

IndrajeetPatil requested a review from lorenzwalthert October 30, 2022 18:19

reference via index, not name

24684f6

this should be more robust against changes in the R parser with the goal to avoid CRAN submission due to failing examples

lorenzwalthert approved these changes Nov 7, 2022

View reviewed changes

lorenzwalthert self-requested a review November 7, 2022 16:18

IndrajeetPatil merged commit 3c779b2 into main Nov 7, 2022

IndrajeetPatil deleted the export_third_party_helpers branch November 7, 2022 16:27

IndrajeetPatil added a commit that referenced this pull request Nov 7, 2022

Include all third-party style guide helpers on website

daf2f83

Follow-up on #1043

IndrajeetPatil mentioned this pull request Nov 7, 2022

Include all third-party style guide helpers on website #1052

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose internals used with other style guides #1043

Expose internals used with other style guides #1043

IndrajeetPatil commented Oct 28, 2022 •

edited

Loading

codecov-commenter commented Oct 28, 2022 •

edited

Loading

github-actions bot commented Oct 28, 2022

Robinlovelace commented Oct 30, 2022

IndrajeetPatil Oct 30, 2022

IndrajeetPatil Oct 30, 2022

IndrajeetPatil Oct 30, 2022

IndrajeetPatil Oct 30, 2022

lorenzwalthert Oct 30, 2022 •

edited

Loading

IndrajeetPatil Nov 2, 2022

IndrajeetPatil commented Oct 30, 2022

github-actions bot commented Oct 30, 2022

Robinlovelace commented Oct 30, 2022

github-actions bot commented Nov 2, 2022

lorenzwalthert commented Nov 2, 2022 •

edited

Loading

lorenzwalthert commented Nov 7, 2022 •

edited

Loading

IndrajeetPatil commented Nov 7, 2022

github-actions bot commented Nov 7, 2022

Expose internals used with other style guides #1043

Expose internals used with other style guides #1043

Conversation

IndrajeetPatil commented Oct 28, 2022 • edited Loading

Progress tracker

codecov-commenter commented Oct 28, 2022 • edited Loading

Codecov Report

github-actions bot commented Oct 28, 2022

Robinlovelace commented Oct 30, 2022

IndrajeetPatil Oct 30, 2022

Choose a reason for hiding this comment

IndrajeetPatil Oct 30, 2022

Choose a reason for hiding this comment

IndrajeetPatil Oct 30, 2022

Choose a reason for hiding this comment

IndrajeetPatil Oct 30, 2022

Choose a reason for hiding this comment

lorenzwalthert Oct 30, 2022 • edited Loading

Choose a reason for hiding this comment

IndrajeetPatil Nov 2, 2022

Choose a reason for hiding this comment

IndrajeetPatil commented Oct 30, 2022

github-actions bot commented Oct 30, 2022

Robinlovelace commented Oct 30, 2022

github-actions bot commented Nov 2, 2022

lorenzwalthert commented Nov 2, 2022 • edited Loading

lorenzwalthert commented Nov 7, 2022 • edited Loading

IndrajeetPatil commented Nov 7, 2022

github-actions bot commented Nov 7, 2022

IndrajeetPatil commented Oct 28, 2022 •

edited

Loading

codecov-commenter commented Oct 28, 2022 •

edited

Loading

lorenzwalthert Oct 30, 2022 •

edited

Loading

lorenzwalthert commented Nov 2, 2022 •

edited

Loading

lorenzwalthert commented Nov 7, 2022 •

edited

Loading