r-lib / debugme

Easy and efficient debugging for R packages
https://r-lib.github.io/debugme/
Other
149 stars 10 forks source link

Make it easier to go into "debug mode" #26

Open jennybc opened 6 years ago

jennybc commented 6 years ago

What would it take to make it easier to turn debugging messages on (and off) during development?

I think just a helper function would be nice versus manually setting an env var. Maybe it could default to the current package?

Then there's also the issue of turning it on, then needing to reload the package. That sounds a bit harder to smooth over, but maybe also possible? I somehow thought I needed to install/restart/reload, but load_all() works, so nevermind.

gaborcsardi commented 6 years ago

Yeah, I agree that it needs to be improved.

gaborcsardi commented 6 years ago

So what I am trying to do is to have an html widget that serves as the log viewer, and debugme would send the log messages to the widget, instead of dumping them to the screen.

krlmlr commented 5 years ago

FWIW, the following zzz.R works for me. The debugme:::.onLoad() call isn't particularly pleasant, and a debugme_sitrep() would be great too:

.onLoad <- function(libname, pkgname) {
  # Necessary to re-parse environment variable
  debugme:::.onLoad(libname, pkgname)

  debugme::debugme()
  debug_info()
}

activate_debugme <- function(bangs = 2) {
  old_debugme <- remove_from_logging(get_debugme())
  old_debugme <- gsub("(.)$", "\\1,", old_debugme)

  my_debugme <- paste0(strrep("!", bangs), get_pkgname())

  set_debugme(paste0(old_debugme, my_debugme))
}

deactivate_debugme <- function() {
  new_debugme <- remove_from_logging(get_debugme())
  set_debugme(new_debugme)
}

get_debugme <- function() {
  Sys.getenv("DEBUGME")
}

set_debugme <- function(debugme) {
  Sys.setenv("DEBUGME" = debugme)
  message("DEBUGME=", debugme)
}

remove_from_logging <- function(spec) {
  spec <- gsub(paste0("!*", get_pkgname(), ""), "", spec)
  spec <- gsub(",,+", ",", spec)
  spec
}

debug_info <- function(pkgname) {
  "!DEBUG `get_pkgname()` loaded"
  "!!DEBUG Two bangs"
  "!!!DEBUG Three bangs"
  "!!!!DEBUG Four bangs"
}

get_pkgname <- function() {
  environmentName(topenv(environment()))
}

Here's an unreprex showing it in action. (I wonder why the bangs don't work, though.)

setwd(".../dynafilter")

devtools::load_all()
#> Loading dynafilter

activate_debugme()
#> DEBUGME=!!dynafilter
devtools::load_all()
#> Loading dynafilter
#> dynafilter dynafilter loaded 
#> dynafilter Two bangs +1ms 
#> dynafilter Three bangs +4ms 
#> dynafilter Four bangs +3ms

activate_debugme(bangs = 3)
#> DEBUGME=!!!dynafilter
devtools::load_all()
#> Loading dynafilter
#> dynafilter dynafilter loaded +509ms 
#> dynafilter Two bangs +4ms 
#> dynafilter Three bangs +5ms 
#> dynafilter Four bangs +4ms

deactivate_debugme()
#> DEBUGME=
devtools::load_all()
#> Loading dynafilter

Created on 2019-06-27 by the reprex package (v0.3.0)

r2evans commented 3 years ago

Long time nothing heard ... I appreciate the simplicity of the package.

I think jennybc's suggestion to make it easier to toggle during development might still need some discussion. For instance, with an .onLoad of just debugme::debugme(), the first load_all works, but if I change the envvar and re-load_all, the new setting is not noticed. krlmlr's suggestion to use debugme:::.onLoad(.) works, though since I push my packages elsewhere, I run R's checks ... and it doesn't really appreciate the ::: use. As a workaround, I've found

.onLoad <- function(libname, pkgname) {
  get(".onLoad", asNamespace("debugme"))(libname, pkgname)
  debugme::debugme()
}

successfully hides it from R checks, though that's only being sneaky, not doing it "the right way".

Would it be a problem to change the recommended action to be

.onLoad <- function(libname, pkgname) {
  debugme::debugme(libname = libname, pkgname = pkgname)
}

or similar, employing whatever steps are required within debug:::.onLoad to re-examine the envvar?


Reprex: a package with:

testme <- function(...) {
  "!DEBUG This is a test of debug level `1`"
  "!!DEBUG This is a test of debug level `1+1`"
  "!!!DEBUG This is a test of debug level `sqrt(9)`"
  TRUE
}
.onLoad <- function(libname, pkgname) {
  debugme::debugme()
}

In a fresh R instance,

Sys.setenv(DEBUGME="!mypackage")
devtools::load_all("mypackage")
# Loading mypackage
# testme()
mypackage This is a test of debug level 1 
# [1] TRUE

Sys.setenv(DEBUGME="!!!mypackage")
devtools::load_all("mypackage")
# Loading mypackage
testme()
# mypackage This is a test of debug level 1 +26281ms 
# [1] TRUE

(detaching the package doesn't help, a little surprising to me.)

However, if I add debugme:::.onLoad (or my R-check-proof variant) and reload, it works:

### edit .onLoad()
devtools::load_all("mypackage")
# Loading mypackage
testme()
# mypackage This is a test of debug level 1 +21978ms 
# mypackage This is a test of debug level 2 +1ms 
# mypackage This is a test of debug level 3 +0ms 
# [1] TRUE

(Could this behavior be due to something in devtools::load_all?)

krlmlr commented 3 years ago

The PR makes it easier but doesn't resolve the original issue. We could add helpers for editing the environment variable, for sure. Here or in usethis?

@r2evans: The env var is now loaded in every debugme() call, one step closer.

r2evans commented 3 years ago

Honestly, I don't see the utility in helper functions for this, since it's simply controlled by an env var. I know some really like helper-funcs, though, I'm not opposed.

gaborcsardi commented 3 years ago

We could add some functions like this:

debug_package(package, level = 1)
undebug_package(package)
debug_status()

But I agree that it is not crucial.

krlmlr commented 3 years ago

Do you mind bringing in cli as a dependency for pretty output?

gaborcsardi commented 3 years ago

Do you mind bringing in cli as a dependency for pretty output?

Nope.

r2evans commented 3 years ago

Are you trying to minimize the footprint? That will include assertthat.

gaborcsardi commented 3 years ago

assertthat is very small and will be included in cli soon.

gaborcsardi commented 3 years ago

Otoh with cli we can drop crayon.