code style and reorganize logging functions

IRkernel · Mar 29, 2016 · dc9fb21 · dc9fb21
1 parent d7361ee
commit dc9fb21
Show file tree

Hide file tree

Showing 8 changed files with 85 additions and 101 deletions.
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -26,6 +26,7 @@ Imports:
     uuid,
     digest
 Collate:
+    'logging.r'
     'comm_manager.r'
     'options.r'
     'execution.r'

diff --git a/R/comm_manager.r b/R/comm_manager.r
@@ -1,3 +1,6 @@
+#' @include logging.r
+NULL
+
 setClassUnion('functionOrNULL', members = c('function', 'NULL'))
 
 #' The Comm_Manager
@@ -16,7 +19,7 @@ Comm_Manager <- setRefClass(
     methods = list(
         new_comm = function(target_name, comm_id = NULL) {
             if(is.null(comm_id)) {
-                comm_id = UUIDgenerate()
+                comm_id <- UUIDgenerate()
             }
             return (Comm$new(id = comm_id, target_name = target_name, comm_manager = .self))
         },
@@ -32,10 +35,10 @@ Comm_Manager <- setRefClass(
         unregister_comm = function(comm) {
             commid_to_comm[[comm$id]] <<- NULL
         },
-        is_comm_registered = function(comm){
+        is_comm_registered = function(comm) {
             return (!is.null(commid_to_comm[[comm$id]]))
         },
-        send_open = function(comm_id, target_name, data, metadata = list()){
+        send_open = function(comm_id, target_name, data, metadata = list()) {
             send_response('comm_open', parent_request, 'iopub', list(
                 metadata = metadata,
                 comm_id = comm_id,
@@ -61,11 +64,8 @@ Comm_Manager <- setRefClass(
         },
         make_comm_list = function(comm_list) {
             all_comms <- list()
-            for(the_comm in comm_list) {
-                the_target_name <- the_comm$target_name
-                the_target_name_list <- list()
-                the_target_name_list[['target_name']] <- the_target_name
-                all_comms[[the_comm$id]] <- the_target_name_list
+            for (the_comm in comm_list) {
+                all_comms[[the_comm$id]] <- list(target_name = the_comm$target_name)
             }
             return (all_comms)
         },
@@ -86,7 +86,7 @@ Comm_Manager <- setRefClass(
             #Else target_name not in the request return all commids accross all targets
             reply_msg <- list()
             comms <- list()
-            if("target_name" %in% names(request$content)) {
+            if('target_name' %in% names(request$content)) {
                 #reply with comms only for the specified target_name
                 target_name_requested <- request$content$target_name
                 filtered_comms <- Filter(function(x) x$target_name == target_name_requested, commid_to_comm)
@@ -117,11 +117,10 @@ Comm_Manager <- setRefClass(
                 tryCatch({
                     target_to_handler_map[[target_name]](comm, data)
                 }, error = function(e) {
-                    debug("error invoking the handler for target")
-                    debug(e)
+                    log_debug('error invoking the handler for target: %s', e)
                 })
             } else {
-                debug("target_name not found in comm_open")
+                log_debug('target_name not found in comm_open')
                 #reply with a comm_close message as target_name not found
                 send_close(comm_id, target_name, list())
             }
@@ -141,11 +140,10 @@ Comm_Manager <- setRefClass(
                 tryCatch({
                     comm$handle_msg(data)
                 }, error = function(e) {
-                    debug("error invoking comm handle msg")
-                    debug(e)
+                    log_debug('error invoking comm handle msg: %s', e)
                 })
             } else {
-                debug("comm_id not found in comm_msg")
+                log_debug('comm_id not found in comm_msg')
             }
         },
 
@@ -162,12 +160,11 @@ Comm_Manager <- setRefClass(
                 tryCatch({
                     comm$handle_close()
                 }, error = function(e) {
-                    debug("error invoking comm handle close")
-                    debug(e)
+                    log_debug('error invoking comm handle close: %s', e)
                 })
                 unregister_comm(comm)
             } else {
-                debug("comm_id not found in comm_msg")
+                log_debug('comm_id not found in comm_msg')
             }
         },
         initialize = function(...) {
@@ -191,27 +188,27 @@ Comm <- setRefClass(
         close_callback = 'functionOrNULL'
     ),
     methods = list(
-        open = function(msg = list()){
+        open = function(msg = list()) {
             if(!comm_manager$is_comm_registered(.self)) {
                 comm_manager$register_comm(.self)
                 comm_manager$send_open(id, target_name, msg)
             } else {
-                debug("Comm already opened!")
+                log_debug('Comm already opened!')
             }
         },
         send = function(msg = list()) {
             if(comm_manager$is_comm_registered(.self)) {
                 comm_manager$send_msg(id, target_name, msg)
             } else {
-                debug("Comm is not opened. Cannot send!")
+                log_debug('Comm is not opened. Cannot send!')
             }
         },
         close = function(msg = list()) {
             if(comm_manager$is_comm_registered(.self)) {
                 comm_manager$send_close(id, target_name, msg)
                 comm_manager$unregister_comm(.self)
             } else {
-                debug("Comm is already closed!")
+                log_debug('Comm is already closed!')
             }
         },
         on_msg = function(a_msg_callback) {

diff --git a/R/execution.r b/R/execution.r
@@ -112,7 +112,7 @@ execute = function(request) {
 
     # .Last doesn’t seem to work, so replicating behavior
     quit <- function(save = 'default', status = 0, runLast = TRUE) {
-        save = switch(save,
+        save <- switch(save,
             default = , yes = TRUE,
             no = FALSE,
             ask = ask('Save workspace image? [y/n/c]: '),
@@ -149,9 +149,6 @@ execute = function(request) {
     err <<- list()
     nframe <- NULL  # find out stack depth in notebook cell
 
-    # handle to stderr if stderr() is sink'ed
-    stderror <- stderr()
-
     tryCatch(evaluate(
         'stop()',
         stop_on_error = 1L,
@@ -180,17 +177,16 @@ execute = function(request) {
         # We only want a short error in the notebook/... because the usual case
         # is just a problem in one of the display methods which are not relevant
         # right now.
-        handle_display_error <- function(e){
+        handle_display_error <- function(e) {
             # This is used with withCallingHandler and only has two additional
             # calls at the end instead of the 3 for tryCatch... (-2 at the end)
             # we also remove the tryCatch and mime2repr stuff at the head of the callstack (+7)
             calls <- head(sys.calls()[-seq_len(nframe + 7L)], -2)
             stack_info <- format_stack(calls)
-            # TODO: replace with proper logging
             msg <- sprintf('ERROR while rich displaying an object: %s\nTraceback:\n%s\n',
                            toString(e),
                            paste(stack_info, collapse='\n'))
-            cat(msg, file = stderror)
+            log_error(msg)
             if (!silent) {
                 send_response('stream', request, 'iopub', list(
                     name = 'stderr',
@@ -227,7 +223,7 @@ execute = function(request) {
                                     # Isolating full html pages (putting them in an iframe)
                                     if (identical(mime, 'text/html')) {
                                         if (grepl("<html.*>", r, ignore.case = TRUE)){
-                                            jupyter_debug("Found full html page: %s", strtrim(r, 100))
+                                            log_debug("Found full html page: %s", strtrim(r, 100))
                                             metadata[[mime]] <- list(isolated = TRUE)
                                         }
                                     }
@@ -236,7 +232,7 @@ execute = function(request) {
                             error=function(x){})
                     }
                 }
-                jupyter_debug("Sending display_data: %s", paste(capture.output(str(data)), collapse = "\n"))
+                log_debug("Sending display_data: %s", paste(capture.output(str(data)), collapse = "\n"))
                 send_response('display_data', request, 'iopub', list(
                     data = data,
                     metadata = metadata))
@@ -260,7 +256,7 @@ execute = function(request) {
             stream(paste(o$message, collapse = ''), 'stderr')
         }
 
-        handle_warning = function(o) {
+        handle_warning <- function(o) {
             call <- if (is.null(o$call)) '' else paste('In', deparse(o$call)[[1]])
             stream(sprintf('Warning message:\n%s: %s', call, o$message), 'stderr')
         }

diff --git a/R/help.r b/R/help.r
@@ -8,9 +8,10 @@
 #' The following can be set/read via \code{options(opt.name = ...)} / \code{getOption('opt.name')}
 #' 
 #' \describe{
-#'   \item{\code{jupyter.in_kernel}}{\code{TRUE} if this code is executed in a running kernel}
+#'   \item{\code{jupyter.log_level}}{1L (errors), 2L (warnings), or 3L (debug). 1L is the default.}
+#'   \item{\code{jupyter.in_kernel}}{\code{TRUE} if this code is executed in a running kernel. Set to pretend being/not being in a kernel}
 #'   \item{\code{jupyter.rich_display}}{Use more than just text display}
-#'   \item{\code{jupyter.result_mimetypes}}{
+#'   \item{\code{jupyter.display_mimetypes}}{
 #'      The formats emitted when any return value is to be displayed
 #'      (default: all mimetypes listed \href{http://ipython.org/ipython-doc/stable/api/generated/IPython.core.formatters.html#IPython.core.formatters.format_display_data}{here}) 
 #'   }