From d1b60704e8607070086e4c23314badc624ce1a86 Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Tue, 14 Apr 2015 15:38:25 +0200
Subject: Next iteration on diagnostics

---
 brep/module     | 36 +++++++++++++++++++++++-------------
 brep/module.cxx | 37 ++++++++++++++++++++++++++++++-------
 brep/search.cxx |  7 +------
 3 files changed, 54 insertions(+), 26 deletions(-)

diff --git a/brep/module b/brep/module
index 0b69aaf..1803ee8 100644
--- a/brep/module
+++ b/brep/module
@@ -33,6 +33,29 @@ namespace brep
     diag_data data;
   };
 
+  // Every module member function that needs to produce any diagnostics
+  // shall begin with:
+  //
+  // MODULE_DIAG;
+  //
+  // This will instantiate the fail, error, warn, info, and trace
+  // diagnostics streams with the function's name.
+  //
+#define MODULE_DIAG                                                     \
+  const fail_mark<server_error> fail (__PRETTY_FUNCTION__);             \
+  const basic_mark error (severity::error,                              \
+                          this->log_writer_,                            \
+                          __PRETTY_FUNCTION__);                         \
+  const basic_mark warn (severity::warn,                                \
+                         this->log_writer_,                             \
+                         __PRETTY_FUNCTION__);                          \
+  const basic_mark info (severity::info,                                \
+                         this->log_writer_,                             \
+                         __PRETTY_FUNCTION__);                          \
+  const basic_mark trace (severity::info,                               \
+                          this->log_writer_,                            \
+                          __PRETTY_FUNCTION__)
+
   // Adaptation of the web::module to our needs.
   //
   class module: public web::module
@@ -44,11 +67,6 @@ namespace brep
     // Diagnostics.
     //
   protected:
-    const basic_mark error;
-    const basic_mark warn;
-    const basic_mark info;
-    const fail_mark<server_error> fail;
-
     // Trace verbosity level.
     //
     // 0 - tracing disabled.
@@ -62,14 +80,6 @@ namespace brep
     template <class F> static void level1 (const F& f) {if (verb_ >= 1) f ();}
     template <class F> static void level2 (const F& f) {if (verb_ >= 2) f ();}
 
-    struct trace_mark_base: basic_mark_base
-    {
-      trace_mark_base (const module* this_, const char* name)
-          : basic_mark_base (severity::trace, this_->log_writer_, name) {}
-    };
-    using trace_mark = diag_mark<trace_mark_base>;
-    using tracer = trace_mark;
-
     // Implementation details.
     //
   protected:
diff --git a/brep/module.cxx b/brep/module.cxx
index 31e5f99..fb33e1e 100644
--- a/brep/module.cxx
+++ b/brep/module.cxx
@@ -47,13 +47,7 @@ namespace brep
   }
 
   module::
-  module ()
-      : error (severity::error, log_writer_),
-        warn (severity::warn, log_writer_),
-        info (severity::info, log_writer_),
-        log_writer_ (bind (&module::write, this, _1))
-  {
-  }
+  module (): log_writer_ (bind (&module::write, this, _1)) {}
 
   void module::
   log_write (diag_data&& d) const
@@ -63,5 +57,34 @@ namespace brep
 
     //@@ Cast log_ to apache::log and write the records.
     //
+
+    //@@ __PRETTY_FUNCTION__ contains a lot of fluff that we probably
+    // don't want in the logs (like return value and argument list;
+    // though the argument list would distinguish between several
+    // overloads). If that's the case, then this is probably the
+    // best place to process the name and convert something like:
+    //
+    // void module::handle(request, response)
+    //
+    // To just:
+    //
+    // module::handle
+    //
+    // Note to someone who is going to implement this: searching for a
+    // space to determine the end of the return type may not work if
+    // the return type is, say, a template id or a pointer to function
+    // type. It seems a more robust approach would be to scan backwards
+    // until we find the first ')' -- this got to be the end of the
+    // function argument list. Now we continue scanning backwards keeping
+    // track of the ')' vs '(' balance (arguments can also be of pointer
+    // to function type). Once we see an unbalanced '(', then we know this
+    // is the beginning of the argument list. Everything between it and
+    // the preceding space is the qualified function name. Good luck ;-).
+    //
+    // If we also use the name in handle() above (e.g., to return to
+    // the user as part of 505), then we should do it there as well
+    // (in which case factoring this functionality into a separate
+    // function seem to make a lot of sense).
+    //
   }
 }
diff --git a/brep/search.cxx b/brep/search.cxx
index 9a0909c..cc3cee2 100644
--- a/brep/search.cxx
+++ b/brep/search.cxx
@@ -13,12 +13,7 @@ namespace brep
   void search::
   handle (request& rq, response& rs)
   {
-    //@@ Could probably have module name in which case this will
-    //   then become:
-    //
-    //   tracer trace (this, "handle");
-    //
-    tracer trace (this, "search::handle");
+    MODULE_DIAG;
 
     const name_values& ps (rq.parameters ());
 
-- 
cgit v1.1