some quick cleanups (#48)

- adds some missing namespace coverage (comment, labelcounter).
- renames WithTrace to SkipCaller for clearer readability.
- updates the comment headers on SkipCaller funcs.

clues.go

@@ -64,9 +64,12 @@ func AddMapTo[K comparable, V any](
 // used to correlate and isolate logs to certain trace branches.
 // AddTrace is only needed for layers that don't otherwise call Add() or
 // similar functions, since those funcs already attach a new node.
-func AddTrace(ctx context.Context) context.Context {
+func AddTrace(
+	ctx context.Context,
+	traceID string,
+) context.Context {
 	nc := nodeFromCtx(ctx, defaultNamespace)
-	return setDefaultNodeInCtx(ctx, nc.trace(""))
+	return setDefaultNodeInCtx(ctx, nc.trace(traceID))
 }
 
 // AddTraceTo stacks a clues node onto this context within the specified
@@ -75,48 +78,48 @@ func AddTrace(ctx context.Context) context.Context {
 // certain trace branches.  AddTraceTo is only needed for layers that don't
 // otherwise call AddTo() or similar functions, since those funcs already
 // attach a new node.
-func AddTraceTo(ctx context.Context, namespace string) context.Context {
+func AddTraceTo(ctx context.Context, traceID, namespace string) context.Context {
 	nc := nodeFromCtx(ctx, ctxKey(namespace))
-	return setNodeInCtx(ctx, namespace, nc.trace(""))
+	return setNodeInCtx(ctx, namespace, nc.trace(traceID))
 }
 
-// AddTraceName stacks a clues node onto this context and uses the provided
-// name for the trace id, instead of a randomly generated hash. AddTraceName
+// AddTraceWith stacks a clues node onto this context and uses the provided
+// name for the trace id, instead of a randomly generated hash. AddTraceWith
 // can be called without additional values if you only want to add a trace marker.
-func AddTraceName(
+func AddTraceWith(
 	ctx context.Context,
-	name string,
+	traceID string,
 	kvs ...any,
 ) context.Context {
 	nc := nodeFromCtx(ctx, defaultNamespace)
 
 	var node *dataNode
 	if len(kvs) > 0 {
 		node = nc.addValues(normalize(kvs...))
-		node.id = name
+		node.id = traceID
 	} else {
-		node = nc.trace(name)
+		node = nc.trace(traceID)
 	}
 
 	return setDefaultNodeInCtx(ctx, node)
 }
 
-// AddTraceNameTo stacks a clues node onto this context and uses the provided
-// name for the trace id, instead of a randomly generated hash. AddTraceNameTo
+// AddTraceWithTo stacks a clues node onto this context and uses the provided
+// name for the trace id, instead of a randomly generated hash. AddTraceWithTo
 // can be called without additional values if you only want to add a trace marker.
-func AddTraceNameTo(
+func AddTraceWithTo(
 	ctx context.Context,
-	name, namespace string,
+	traceID, namespace string,
 	kvs ...any,
 ) context.Context {
 	nc := nodeFromCtx(ctx, ctxKey(namespace))
 
 	var node *dataNode
 	if len(kvs) > 0 {
 		node = nc.addValues(normalize(kvs...))
-		node.id = name
+		node.id = traceID
 	} else {
-		node = nc.trace(name)
+		node = nc.trace(traceID)
 	}
 
 	return setNodeInCtx(ctx, namespace, node)
@@ -126,6 +129,8 @@ func AddTraceNameTo(
 // comments
 // ---------------------------------------------------------------------------
 
+// AddComment adds a long form comment to the clues.
+//
 // Comments are special case additions to the context.  They're here to, well,
 // let you add comments!  Why?  Because sometimes it's not sufficient to have a
 // log let you know that a line of code was reached. Even a bunch of clues to
@@ -156,6 +161,38 @@ func AddComment(
 	return setDefaultNodeInCtx(ctx, nn)
 }
 
+// AddCommentTo adds a long form comment to the clues in a certain namespace.
+//
+// Comments are special case additions to the context.  They're here to, well,
+// let you add comments!  Why?  Because sometimes it's not sufficient to have a
+// log let you know that a line of code was reached. Even a bunch of clues to
+// describe system state may not be enough.  Sometimes what you need in order
+// to debug the situation is a long-form explanation (you do already add that
+// to your code, don't you?).  Or, even better, a linear history of long-form
+// explanations, each one building on the prior (which you can't easily do in
+// code).
+//
+// Should you transfer all your comments to clues?  Absolutely not.  But in
+// cases where extra explantion is truly important to debugging production,
+// when all you've got are some logs and (maybe if you're lucky) a span trace?
+// Those are the ones you want.
+//
+// Unlike other additions, which are added as top-level key:value pairs to the
+// context, comments are all held as a single array of additions, persisted in
+// order of appearance, and prefixed by the file and line in which they appeared.
+// This means comments are always added to the context and never clobber each
+// other, regardless of their location.  IE: don't add them to a loop.
+func AddCommentTo(
+	ctx context.Context,
+	namespace, msg string,
+	vs ...any,
+) context.Context {
+	nc := nodeFromCtx(ctx, ctxKey(namespace))
+	nn := nc.addComment(1, msg, vs...)
+
+	return setNodeInCtx(ctx, namespace, nn)
+}
+
 // ---------------------------------------------------------------------------
 // hooks
 // ---------------------------------------------------------------------------
@@ -171,3 +208,19 @@ func AddLabelCounter(ctx context.Context, counter Adder) context.Context {
 
 	return setDefaultNodeInCtx(ctx, nn)
 }
+
+// AddLabelCounterTo embeds an Adder interface into this context. Any already
+// embedded Adder will get replaced.  When adding Labels to a clues.Err the
+// LabelCounter will use the label as the key for the Add call, and increment
+// the count of that label by one.
+func AddLabelCounterTo(
+	ctx context.Context,
+	namespace string,
+	counter Adder,
+) context.Context {
+	nc := nodeFromCtx(ctx, ctxKey(namespace))
+	nn := nc.addValues(nil)
+	nn.labelCounter = counter
+
+	return setNodeInCtx(ctx, namespace, nn)
+}

clues_test.go

@@ -246,7 +246,7 @@ func TestAddTrace(t *testing.T) {
 			check := msa{}
 			mustEquals(t, check, clues.In(ctx).Map(), true)
 
-			ctx = clues.AddTrace(ctx)
+			ctx = clues.AddTrace(ctx, "")
 
 			assert(
 				t, ctx, "",
@@ -278,7 +278,7 @@ func TestAddTraceName(t *testing.T) {
 			mustEquals(t, msa{}, clues.In(ctx).Map(), true)
 
 			for _, name := range test.names {
-				ctx = clues.AddTraceName(ctx, name, test.kvs...)
+				ctx = clues.AddTraceWith(ctx, name, test.kvs...)
 			}
 
 			assert(
@@ -376,7 +376,7 @@ func TestAddTraceTo(t *testing.T) {
 			check := msa{}
 			mustEquals(t, check, clues.InNamespace(ctx, "ns").Map(), true)
 
-			ctx = clues.AddTraceTo(ctx, "ns")
+			ctx = clues.AddTraceTo(ctx, "", "ns")
 
 			assert(
 				t, ctx, "ns",
@@ -408,7 +408,7 @@ func TestAddTraceNameTo(t *testing.T) {
 			mustEquals(t, msa{}, clues.InNamespace(ctx, "ns").Map(), true)
 
 			for _, name := range test.names {
-				ctx = clues.AddTraceNameTo(ctx, name, "ns", test.kvs...)
+				ctx = clues.AddTraceWithTo(ctx, name, "ns", test.kvs...)
 			}
 
 			assert(
@@ -630,7 +630,7 @@ func TestAddComment(t *testing.T) {
 	}
 }
 
-func addCommentTo(ctx context.Context, msg string) context.Context {
+func addCommentToCtx(ctx context.Context, msg string) context.Context {
 	return clues.AddComment(ctx, msg)
 }
 
@@ -670,15 +670,15 @@ func commentMatches(
 func TestAddComment_trace(t *testing.T) {
 	ctx := context.Background()
 	ctx = clues.AddComment(ctx, "one")
-	ctx = addCommentTo(ctx, "two")
+	ctx = addCommentToCtx(ctx, "two")
 	ctx = clues.AddComment(ctx, "three")
 
 	dn := clues.In(ctx)
 	comments := dn.Comments()
 	stack := comments.String()
 	expected := commentRE(
 		"TestAddComment_trace", "clues_test.go", "one",
-		"addCommentTo", "clues_test.go", "two",
+		"addCommentToCtx", "clues_test.go", "two",
 		"TestAddComment_trace", "clues_test.go", `three$`)
 
 	commentMatches(t, expected, stack)

err.go

@@ -134,14 +134,15 @@ func makeStack(
 // TODO: transform all this to comply with a standard interface
 // ------------------------------------------------------------
 
-// TODO: this will need some cleanup in a follow-up PR.
-//
 // ancestors builds out the ancestor lineage of this
 // particular error.  This follows standard layout rules
 // already established elsewhere:
 // * the first entry is the oldest ancestor, the last is
 // the current error.
 // * Stacked errors get visited before wrapped errors.
+//
+// TODO: get other ancestor stack builders to work off of this
+// func instead of spreading that handling everywhere.
 func ancestors(err error) []error {
 	return stackAncestorsOntoSelf(err)
 }
@@ -847,20 +848,25 @@ func WithMap(err error, m map[string]any) *Err {
 // builders - tracing
 // ------------------------------------------------------------
 
-// WithTrace sets the error trace to a certain depth.
-// A depth of 0 traces to the func where WithTrace is
-// called.  1 sets the trace to its parent, etc.
-// Error traces are already generated for the location
-// where clues.Wrap or clues.Stack was called.  This
-// call is for cases where Wrap or Stack calls are handled
-// in a helper func and are not reporting the actual
-// error origin.
-// TODO: rename to `SkipCaller`.
-func (err *Err) WithTrace(depth int) *Err {
+// SkipCaller skips <depth> callers when constructing the
+// error trace stack.  The caller is the file, line, and func
+// where the *clues.Err was generated.
+//
+// A depth of 0 performs no skips, and returns the same
+// caller info as if SkipCaller was not called.  1 skips the
+// immediate parent, etc.
+//
+// Error traces are already generated for the location where
+// clues.Wrap or clues.Stack was called.  This func is for
+// cases where Wrap or Stack calls are handled in a helper
+// func and are not reporting the actual error origin.
+func (err *Err) SkipCaller(depth int) *Err {
 	if isNilErrIface(err) {
 		return nil
 	}
 
+	// needed both here and in withTrace() to
+	// correct for the extra call depth.
 	if depth < 0 {
 		depth = 0
 	}
@@ -870,18 +876,22 @@ func (err *Err) WithTrace(depth int) *Err {
 	return err
 }
 
-// WithTrace sets the error trace to a certain depth.
-// A depth of 0 traces to the func where WithTrace is
-// called.  1 sets the trace to its parent, etc.
-// Error traces are already generated for the location
-// where clues.Wrap or clues.Stack was called.  This
-// call is for cases where Wrap or Stack calls are handled
-// in a helper func and are not reporting the actual
-// error origin.
+// SkipCaller skips <depth> callers when constructing the
+// error trace stack.  The caller is the file, line, and func
+// where the *clues.Err was generated.
+//
+// A depth of 0 performs no skips, and returns the same
+// caller info as if SkipCaller was not called.  1 skips the
+// immediate parent, etc.
+//
+// Error traces are already generated for the location where
+// clues.Wrap or clues.Stack was called.  This func is for
+// cases where Wrap or Stack calls are handled in a helper
+// func and are not reporting the actual error origin.
+//
 // If err is not an *Err intance, returns the error wrapped
 // into an *Err struct.
-// TODO: rename to `SkipCaller`.
-func WithTrace(err error, depth int) *Err {
+func WithSkipCaller(err error, depth int) *Err {
 	if isNilErrIface(err) {
 		return nil
 	}
@@ -897,7 +907,7 @@ func WithTrace(err error, depth int) *Err {
 		return newErr(err, "", map[string]any{}, depth+1)
 	}
 
-	return e.WithTrace(depth + 1)
+	return e.SkipCaller(depth + 1)
 }
 
 // ------------------------------------------------------------

err_test.go

@@ -1340,11 +1340,11 @@ func TestLabelCounter_variadic_noCluesInErr(t *testing.T) {
 // ---------------------------------------------------------------------------
 
 func withTraceWrapper(err error, depth int) error {
-	return clues.WithTrace(err, depth)
+	return clues.WithSkipCaller(err, depth)
 }
 
 func cluesWithTraceWrapper(err *clues.Err, depth int) error {
-	return err.WithTrace(depth)
+	return err.SkipCaller(depth)
 }
 
 func withCommentWrapper(