styling with styler #170

lorenzwalthert · 2017-08-30T14:46:08Z

Note that I have used style_dir("R") and style_dir("tests/testthat", recursive = FALSE) to avoid styling in.R files. 😄

lorenzwalthert · 2017-08-30T14:46:57Z

R/modify_pd.R

+                        special_token,
+                        "LEFT_ASSIGN",
+                        "'$'"
+                      )) {


@krlmlr desired?

Looks good to me.

lorenzwalthert · 2017-08-30T14:47:21Z

R/reindent.R

-      nrow(pd_nested) > 3 &&
-      pd_nested$lag_newlines[first_non_comment_after_call] == 0) {
+    nrow(pd_nested) > 3 &&
+    pd_nested$lag_newlines[first_non_comment_after_call] == 0) {


Looks good.

lorenzwalthert · 2017-08-30T14:47:47Z

R/reindent.R

@@ -58,7 +58,7 @@ update_indention_ref_fun_call <- function(pd_nested) {
 #' }
 update_indention_ref_fun_dec <- function(pd_nested) {
  if (pd_nested$token[1] == "FUNCTION" &&
-      nrow(pd_nested) > 3) {
+    nrow(pd_nested) > 3) {
    seq <- 3:(nrow(pd_nested) - 1)


lorenzwalthert · 2017-08-30T14:48:14Z

R/rules-line_break.R

-  pd$lag_newlines[setdiff(3, exception_pos)] <- 1L
-  pd
-}
+    if (!is_function_call(pd)) return(pd)


Indention desired if function() is on new line?

See line 66.

Not really, but we can postpone. For now, maybe move function one line up?

the problem is that the function name is so long that it does not all fit on one line...

lorenzwalthert · 2017-08-30T14:50:45Z

R/serialized_tests.R

-    # transformer functions
-    filler     = create_filler,
+  transformers <- list( # transformer functions
+    filler = create_filler,


moves comment up?! Probably undesired.

Fixed that in some branch, is a minor thing.

lorenzwalthert · 2017-08-30T14:51:20Z

R/style_guides.R

@@ -156,8 +156,7 @@ create_style_guide <- function(filler = create_filler,
                               token = NULL,
                               indention = NULL,
                               use_raw_indention = FALSE) {
-  lst(
-    # transformer functions
+  lst( # transformer functions


Again, comment was moved up.

lorenzwalthert · 2017-08-30T14:51:57Z

R/token.R

+  "-", "math", "'-'",
+  "*", "math", "'*'",
+  "/", "math", "'/'",
+  "^", "math", "'^'"
 )


will revert that.

lorenzwalthert · 2017-08-30T14:53:56Z

tests/testthat/test-indention_curly.R

+  "complete styling via top level api is correct",
+  "(round, curly, spacing)"
+), {
+  expect_warning(test_collection(


Not sure about that.

Put the description on one line, make it shorter if necessary ;-)

Otherwise, the correct styling would be:

test_that( paste( ... ), { ... } )

Reference: #125 (comment).

lorenzwalthert · 2017-08-30T14:55:03Z

tests/testthat/test-indention_operators.R

+      NA
+    )
+  }
+)


that looks strange.

@krlmlr would you agree that there should be no line break if the token before ) is }?

I think that would comply with section 2.4, although function calls are not explicitly mentioned there.

Fixed that in some branch, is a minor thing.

This looks correct, except the { should be on its own line.

Reference: #125 (comment).

codecov · 2017-08-30T14:59:15Z

Codecov Report

Merging #170 into master will increase coverage by 0.04%.
The diff coverage is 95.03%.

@@            Coverage Diff             @@
##           master     #170      +/-   ##
==========================================
+ Coverage   93.85%   93.89%   +0.04%     
==========================================
  Files          19       19              
  Lines         748      786      +38     
==========================================
+ Hits          702      738      +36     
- Misses         46       48       +2

Impacted Files	Coverage Δ
R/ws.R	`88.88% <ø> (ø)`	⬆️
R/parsed.R	`93.75% <ø> (ø)`	⬆️
R/modify_pd.R	`92.68% <ø> (ø)`	⬆️
R/serialized_tests.R	`100% <100%> (ø)`	⬆️
R/unindent.R	`96.42% <100%> (+0.27%)`	⬆️
R/relevel.R	`100% <100%> (ø)`	⬆️
R/nested.R	`98.38% <100%> (+0.11%)`	⬆️
R/visit.R	`100% <100%> (ø)`	⬆️
R/transform.R	`96.72% <100%> (+0.11%)`	⬆️
R/rules-other.R	`100% <100%> (ø)`	⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ad76a7...a82e2a0. Read the comment docs.

manually restoring line breaks and and styling afterwards (lines not broken automatically anymore).

krlmlr · 2017-08-30T17:00:25Z

R/modify_pd.R

+                        special_token,
+                        "LEFT_ASSIGN",
+                        "'$'"
+                      )) {


Looks good to me.

krlmlr · 2017-08-30T17:03:38Z

R/reindent.R

-      nrow(pd_nested) > 3 &&
-      pd_nested$lag_newlines[first_non_comment_after_call] == 0) {
+    nrow(pd_nested) > 3 &&
+    pd_nested$lag_newlines[first_non_comment_after_call] == 0) {


Looks good.

krlmlr · 2017-08-30T17:06:44Z

R/rules-line_break.R

-  pd$lag_newlines[setdiff(3, exception_pos)] <- 1L
-  pd
-}
+    if (!is_function_call(pd)) return(pd)


Not really, but we can postpone. For now, maybe move function one line up?

krlmlr · 2017-08-30T17:13:02Z

tests/testthat/test-indention_curly.R

+  "complete styling via top level api is correct",
+  "(round, curly, spacing)"
+), {
+  expect_warning(test_collection(


Put the description on one line, make it shorter if necessary ;-)

Otherwise, the correct styling would be:

test_that( paste( ... ), { ... } )

krlmlr · 2017-08-30T17:13:53Z

tests/testthat/test-indention_operators.R

+      NA
+    )
+  }
+)


This looks correct, except the { should be on its own line.

styling with styler

ee814d1

lorenzwalthert commented Aug 30, 2017

View reviewed changes

re-align.

d820237

lorenzwalthert commented Aug 30, 2017

View reviewed changes

lorenzwalthert requested a review from krlmlr August 30, 2017 14:56

restoring line breaks

a82e2a0

manually restoring line breaks and and styling afterwards (lines not broken automatically anymore).

lorenzwalthert force-pushed the tidyup branch from 348440f to a82e2a0 Compare August 30, 2017 15:42

krlmlr approved these changes Aug 30, 2017

View reviewed changes

lorenzwalthert closed this Sep 17, 2017

lorenzwalthert deleted the tidyup branch September 17, 2017 20:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

styling with styler #170

styling with styler #170

lorenzwalthert commented Aug 30, 2017 •

edited

Loading

lorenzwalthert Aug 30, 2017

krlmlr Aug 30, 2017

lorenzwalthert Aug 30, 2017 •

edited

Loading

krlmlr Aug 30, 2017

lorenzwalthert Aug 30, 2017

lorenzwalthert Aug 30, 2017

lorenzwalthert Aug 30, 2017

krlmlr Aug 30, 2017

lorenzwalthert Aug 30, 2017

lorenzwalthert Aug 30, 2017

lorenzwalthert Aug 30, 2017

lorenzwalthert Aug 30, 2017

lorenzwalthert Aug 30, 2017

lorenzwalthert Aug 30, 2017

krlmlr Aug 30, 2017

lorenzwalthert Aug 30, 2017

lorenzwalthert Aug 30, 2017

lorenzwalthert Aug 30, 2017

lorenzwalthert Aug 30, 2017

lorenzwalthert Aug 30, 2017

krlmlr Aug 30, 2017

lorenzwalthert Aug 30, 2017

codecov bot commented Aug 30, 2017 •

edited

Loading

krlmlr Aug 30, 2017

krlmlr Aug 30, 2017

krlmlr Aug 30, 2017

krlmlr Aug 30, 2017

krlmlr Aug 30, 2017

styling with styler #170

styling with styler #170

Conversation

lorenzwalthert commented Aug 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lorenzwalthert Aug 30, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Aug 30, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lorenzwalthert commented Aug 30, 2017 •

edited

Loading

lorenzwalthert Aug 30, 2017 •

edited

Loading

codecov bot commented Aug 30, 2017 •

edited

Loading