Remove leading blank lines when strict = TRUE

[IndrajeetPatil]

Closes #1014

[IndrajeetPatil]

@lorenzwalthert I think the PR is now set, except for one thing: I can't figure out why the cache-related tests are failing. I've commented them out for now.

[lorenzwalthert]

Ok, I'll have a look.

[lorenzwalthert]

Yeah, that's indeed a bit tricky... Seems like on first round, line breaks are removed, but then in the second round, they are added again...

    devtools::load_all()
#> ℹ Loading styler
#> In tests/testthat/helpers-devel-options: Deactivated cache.
    test_that("When expressions are cached, number of newlines between them are preserved", {
    
    local_test_setup(cache = TRUE)
    text <- c(
        "1 + 1",
        "",
        "",
        "f(x)",
        "",
        "",
        "",
        "x < 3",
        "function() NULL"
    )
    # add to cache
    expect_equal(text[1:4], as.character(style_text(text[1:4])))
    # applied cache
    expect_equal(text[1:4], as.character(style_text(text[1:4])))
    
    cat(as.character(style_text(text)), sep = "\n")
    cat('-----------------------------------------\n')
    cat(as.character(style_text(text)), sep = "\n")
})
#> 1 + 1
#> 
#> 
#> f(x)
#> x < 3
#> function() NULL
#> -----------------------------------------
#> 1 + 1
#> 
#> 
#> f(x)
#> 
#> 
#> 
#> x < 3
#> function() NULL
#> Test passed 😀

^{Created on 2022-11-21 with reprex v2.0.2}

[IndrajeetPatil]

Did you comment on this PR? I saw an email about it, but can't find it here.

[lorenzwalthert]

yeah, I deleted it again because I realised it was gibberish :D. Will have to dig in deeper.

[codecov-commenter]

Codecov Report

Merging #1056 (e143717) into main (509bea1) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1056      +/-   ##
==========================================
+ Coverage   91.05%   91.08%   +0.02%     
==========================================
  Files          46       46              
  Lines        2684     2692       +8     
==========================================
+ Hits         2444     2452       +8     
  Misses        240      240              

Impacted Files	Coverage Δ
R/roxygen-examples.R	98.43% <100.00%> (+0.05%)	⬆️
R/transform-block.R	100.00% <100.00%> (ø)
R/transform-files.R	97.48% <100.00%> (+0.06%)	⬆️

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

[github-actions]

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

•   :ballot_box_with_check:cache_applying: 36.4ms -> 36.9ms [-0.98%, +3.27%]
•   :ballot_box_with_check:cache_recording: 1.23s -> 1.22s [-1.44%, +0.63%]
• ❗🐌without_cache: 3.08s -> 3.12s [+0.25%, +2.23%]

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

[lorenzwalthert]

Sorry @IndrajeetPatil that I did not have time to review this. I currently have a lot of (more urgent) things that keep me busy and I first need to understand whats going on... Thanks for your patience.

[github-actions]

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

• ❗🐌cache_applying: 27.3ms -> 27.5ms [+0.16%, +1.31%]
•   :ballot_box_with_check:cache_recording: 859ms -> 859ms [-0.74%, +0.73%]
•   :ballot_box_with_check:without_cache: 2.12s -> 2.12s [-0.59%, +0.19%]

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

[github-actions]

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

•   :ballot_box_with_check:cache_applying: 55ms -> 55.3ms [-0.2%, +1.33%]
•   :ballot_box_with_check:cache_recording: 967ms -> 969ms [-0.3%, +0.6%]
•   :ballot_box_with_check:without_cache: 2.29s -> 2.29s [-0.3%, +0.41%]

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

[lorenzwalthert]

Ok, I finally got a chance to look at this... I described the solution in the code. Another question is, if and where we want to remove additional blank lines between expressions. Status quo is not to remove any I think.

[IndrajeetPatil]

if and where we want to remove additional blank lines between expressions. Status quo is not to remove any I think

That was definitely an unwanted change. I think the current behaviour on main branch is the correct behaviour.

[github-actions]

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

•   :ballot_box_with_check:cache_applying: 44.2ms -> 44.7ms [-0.82%, +3.02%]
•   :ballot_box_with_check:cache_recording: 771ms -> 773ms [-0.48%, +1.24%]
•   :ballot_box_with_check:without_cache: 1.83s -> 1.83s [-0.19%, +0.62%]

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