Few issues solved - separator/DRY/list(...)

[Polkas]

I think it is enough for this PR, new ideas I will push in another one.

• [x] separator https://github.com/daroczig/logger/issues/95 After many tests I think that the new utils function catch_base_log is the best way to get log header length (nchar). It might return wrong number of chars if log header has different length under different environments. Another idea which was not implemented is to add width argument to logger -> log_level. Thus control message/output length at the creation level. I provide informative roxygen2 docs when log_separator is not perfectly supported.
• [x] list(...) https://github.com/daroczig/logger/issues/92 For list(..) I transfer part of the code.
• [x] DRY rule for log_* family functions - a new template function log_base
• [x] ifelse -> if else - where input is singular. ifelse is recommended for vectors where as for single logical we should use if else

[daroczig]

sorry for the delay with this - got distracted by #98 and the weekend .. I will pick it up again in a few days

[daroczig]

This is great, thank you very much again! Also thanks for the ping in the other PR on the Travis -> GH actions update :+1:

I am ready to merge this, but may I ask the below changes before that?

• Can you please revert the ifelse updates? I don't see a clear winner at the time being, so let's delay that discussion to a future PR.
• Can you please revert the code style changes in the old code? E.g. breaking long roxygen lines. And update the newly added lines to conform with the rest of the codebase: single quotes, 4 spaces.

No problem if not, I can totally do that, just let me know.

[Polkas]

I reverted ifelse, " quote and roxygen2 long lines split. Moreover I go through new code and update it to use 4 spaces.

Have a great new year:)

[codecov-commenter]

Codecov Report

Merging #96 (98f0f09) into master (ca896c7) will increase coverage by 1.65%. The diff coverage is 100.00%.

:exclamation: Current head 98f0f09 differs from pull request most recent head e9c6acf. Consider uploading reports for the commit e9c6acf to get more accurate results

@@            Coverage Diff             @@
##           master      #96      +/-   ##
==========================================
+ Coverage   79.11%   80.76%   +1.65%     
==========================================
  Files          10       10              
  Lines         522      520       -2     
==========================================
+ Hits          413      420       +7     
+ Misses        109      100       -9     

Impacted Files	Coverage Δ
R/helpers.R	100.00% <100.00%> (ø)
R/logger.R	98.85% <100.00%> (+7.70%)	:arrow_up:
R/utils.R	100.00% <100.00%> (ø)

---

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 ca896c7...e9c6acf. Read the comment docs.

[Polkas]

Luckily I have a pythonanywhere account, with a Linux and the old R version (3.6.3).

Looks like we find some edge case.

should be "TEMP/nchar INFO: " and we get "TEMP/eval INFO: " (.topcall == eval(expr, pf) not nchar(logger:::catch_base_log(...)))

[daroczig]

I am pretty sure that is due to testtat messing with the scopes, see e.g. https://github.com/daroczig/logger/blob/master/tests/testthat/test-CRANSKIP-logger-namespaces.R#L18

One workaround, just like above, is making the call happen outside of testthat .. or just remove the fn from the check as well :zipper_mouth_face:

[Polkas]

Finally I removed one line (one test), as I think we should still test how.topcall influence the call length. If you prefer other solution, of course I am leaving it to you.)