ryapric / loggit

Modern Logging for the R Ecosystem
Other
37 stars 2 forks source link

loggit() messaging to console suppressed during R markdown rendering due to use of cat() #17

Open hlau-mdsol opened 2 years ago

hlau-mdsol commented 2 years ago

In the current loggit.R, echoing of log message is handled by write_ndjson()

write_ndjson(log_df, echo = echo) https://github.com/ryapric/loggit/blob/5399852eed343fba22d4bfc488a4a950f138e414/R/loggit.R#L83

which in turn calls cat() if echo = T. if (echo) cat(logdata, sep = "\n") https://github.com/ryapric/loggit/blob/5399852eed343fba22d4bfc488a4a950f138e414/R/json.R#L102

Previously, in version 1.1.1 (https://cran.r-project.org/src/contrib/Archive/loggit/loggit_1.1.1.tar.gz), if echo = T, the base message function is called in loggit.R. if (echo) base::message(paste(c(log_lvl, log_msg), collapse = ": "))

Switching to from message() to cat() causes loggit output to console to be suppressed during R markdown rendering as knitr::knit_hooks has options to handle message() output but nothing to handle cat() output (https://bookdown.org/yihui/rmarkdown-cookbook/output-hooks.html).

Here is an example code snippet (I replaced the markdown backticks with a single quote because I can't figure out a way not to confused the code blocking):

'''{r setup, include=FALSE} library(loggit) ''' '''{r test message, echo=F,message=F, warning=F} loggit("INFO", "loggit message", echo = T) message('base message\n') '''

Only "base message" will be printed to console during rendering.

I suggest switching back from cat() to message() when echo = T.

ryapric commented 2 years ago

Thanks! Interesting use case I'd not heard of, but it makes sense.

Have you reviewed the release notes for v2.0.0, that changed how loggit handles log formats? Using cat() vs. base::message() is a consequence of that.

I can take a look, but I recall that change being deliberate. If you can get a working PR together that still passes all the tests, I'm happy to review.

ryapric commented 2 years ago

@hlau-mdsol please try version 2.1.2 (which is up on the develop branch), and let me know if that now works as expected.