debugme icon indicating copy to clipboard operation
debugme copied to clipboard

Make it easier to go into "debug mode"

Open jennybc opened this issue 8 years ago • 12 comments

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.

jennybc avatar Nov 16 '17 23:11 jennybc

Yeah, I agree that it needs to be improved.

gaborcsardi avatar Nov 22 '17 09:11 gaborcsardi

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.

gaborcsardi avatar Nov 22 '17 09:11 gaborcsardi

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)

krlmlr avatar Jun 27 '19 13:06 krlmlr

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?)

r2evans avatar Feb 04 '21 21:02 r2evans

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.

krlmlr avatar Feb 17 '21 13:02 krlmlr

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.

r2evans avatar Feb 17 '21 13:02 r2evans

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.

gaborcsardi avatar Feb 17 '21 13:02 gaborcsardi

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

krlmlr avatar Feb 17 '21 17:02 krlmlr

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

Nope.

gaborcsardi avatar Feb 17 '21 17:02 gaborcsardi

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

r2evans avatar Feb 17 '21 18:02 r2evans

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

gaborcsardi avatar Feb 17 '21 18:02 gaborcsardi

Otoh with cli we can drop crayon.

gaborcsardi avatar Feb 17 '21 18:02 gaborcsardi