Don't add pointer comments, or at least let the user disable them

[purpleidea]

When printing with Sdump, we sometimes see these pointer comments:

// p0

and so on... This makes it more difficult to diff. Here's a workaround, but it would be nice to have an option to disable them:

pattern := regexp.MustCompile(`\ \/\/\ p[0-9]+$`) // the p0, p1 comments...
clean := func(s string) string {
	lines := []string{}
	for _, line := range strings.Split(s, "\n") {
		s := pattern.ReplaceAllLiteralString(line, "")
		lines = append(lines, s)
	}
	return strings.Join(lines, "\n")
}

lo1 := clean(lo.Sdump(exp))
lo2 := clean(lo.Sdump(ast))
if lo1 == lo2 { // simple diff
	return
}

I use this to diff two AST's in tests in https://github.com/purpleidea/mgmt/

Thanks!

[atombender]

You can disable them with the DisablePointerReplacement option. However, you will then get duplicate data — not sure if that's a problem for you?

[purpleidea]

As you can see, I am using the DisablePointerReplacement option, and the comments still appear.

purpleidea/mgmt@d7ecc72#diff-ae0ef0944b296aac81d71d5217e3acc998acd6fd6ca73aee7706ffe24192f8ccR2235

[atombender]

I guess we still include the comment even when there's no circular relationship. I'll look into disabling the comments.

[atombender]

The challenge here is what to provide instead when there is a cyclic reference. For example, if you have a cyclic reference today, you get:

&Itself{ // p0
  Value: p0,
}

Without this comment, the p0 would not be defined and we can't provide a replacement. Suggestions?

[purpleidea]

My use case is that your library is fantastic for tests and running a diff between expected and actual AST's for example.

So in my situation there are no cyclic references. In this situation there should be no comments at all. This way the diff will work correctly.

[atombender]

I understand. It's just that we're targeting all kinds of use cases. Unfortunately, we cannot know ahead of time if a pointer is going to be recursive, so we can't make an exception for that case. I suppose we could dump out something like /* recursive pointer */.

[purpleidea]

I suspect a flag to disable this line:

litter/dump.go

Line 90 in 4fde30c

s.write([]byte(fmt.Sprintf(" // %s\n", ptr.label())))

would probably be sufficient.

But I didn't test this yet.

[atombender]

Yes, but then you still end up with the pointer replacements. In a recursive structure, it will say:

&Itself{
  Value: p0,
}

…and p0 isn't "defined" anywhere.

[purpleidea]

I agree, this flag is useful for the most common scenario where there aren't recursive references.