kylelemons/godebug

Debugging helper utilities for Go

godebug's Issues

diff.Diff optimization if one input is empty.

godebug/diff/diff.go

Lines 37 to 40 in e693023

	// Diff returns a string containing a line-by-line unified diff of the linewise
	// changes required to make A into B. Each line is prefixed with '+', '-', or
	// ' ' to indicate if it should be added, removed, or is correct respectively.
	func Diff(A, B string) string {

It seems if one input of Diff is empty, then the algorithm uses O(N^2) space, where N is the size of the other input.

The result is I see out of memory error if I diff an empty chunk with a non-empty chunk.

It'll be a good improvement if we can just do an empty check before calling the Diff algorithm. And generating diffstring in such case should not be very hard.

DiffChunks() with empty slices fails

DiffChunks([]string{}, []string{}) panics with "index out of range".

DiffChunks([]string{"foo"}, []string{}) emits an extra empty chunk[0] in the result.

0001-Failing-test-cases-for-DiffChunk-of-nil.patch.txt

Add support for fmt.GoStringer interface

Apart from fmt.Stringer, it would be good if fmt.GoStringer was supported

I think GoStringer should (at least by default) have higher priority than fmt.Stringer

diff.Diff() with an empty value produces one blank line

I have a series of blobs to diff, each against the previous. The seed is an empty string. Diffing the first blob against that produces a diff with one "removed" line.

-
+{
+  "apiVersion": "v1",
+  "kind": "Service",
+  "metadata": {

pretty.Compare can result in unexpected type coercion

There's some unexpected type coercion that can happen when comparing maps.

package main

import (
	"testing"

	"github.com/kylelemons/godebug/pretty"
)

func TestDiffStringKey(t *testing.T) {
	intKey := map[interface{}]interface{}{
		int(123): "abc",
	}

	stringKey := map[interface{}]interface{}{
		"123": "abc",
	}

	diff := pretty.Compare(intKey, stringKey)
	if len(diff) == 0 {
		t.Fatal("No diff found")
	}
}

func TestDiffFloatKey(t *testing.T) {
	intKey := map[interface{}]interface{}{
		int(123): "abc",
	}

	floatKey := map[interface{}]interface{}{
		float64(123.0): "abc",
	}

	diff := pretty.Compare(intKey, floatKey)
	if len(diff) == 0 {
		t.Fatal("No diff found")
	}
}

Both of these tests will fail. Given the below test also fails, I'm not sure how trivial this will be to correct.

func TestDiffFloat(t *testing.T) {
	diff := pretty.Compare(int(123), float64(123.0))
	if len(diff) != 0 {
		t.Fatal("No diff found")
	}
}

Fixing the first test should be as simple as quoting strings when they're map keys. The second one might require actual type comparison, I'm unsure.

Incompatibility with time.Time

A simple example:

package main

import (
	"fmt"
	"time"

	"github.com/kylelemons/godebug/pretty"
)

func main() {
	now := time.Now()

	pretty.Print(now)
	fmt.Println(now)
}

Result:

{}
2016-11-07 15:46:50.656600439 -0600 CST

Looks like the formatter isn't sure what to do with a time.Time struct.

Compare() function uses a lot of RAM

Because compare first converts the inputs to strings and then uses a diff library, it uses an absolutely enormous amount of ram, making it unusable for some intense purposes.

I'm probably going to have to write my own such Compare function for my purposes 'cause I can't find one, so if I do that I'll definitely submit a PR.

Compare(map, uninitialised map) returns no output

var xs struct{
    x, y  map[string]string
}
xs.x = make(map[string]string)
fmt.Printf("%s", pretty.Compare(xs.x, xs.y)) // returns an empty string

pretty doesn't respect method String() string

type Int int

func (r Int) String() string {
    return fmt.Sprintf("Int(%d)", int(r))
}

type Box struct{ I Int }

func main() {
    i := Int(73)
    fmt.Printf("%+v\t%s\n", i, pretty.Sprint(i))

    b := Box{i}
    fmt.Printf("%+v\t%s\n", b, pretty.Sprint(b))
}

Expected result:

Int(73) Int(73)
{I:Int(73)} {I: Int(73)}

Actual result:

Int(73) 73
{I:Int(73)} {I: 73}

Inifinite recursion issues.

I have some data structures with lots of pointers, back pointers, and the like. Due to all these pointers, there are cycles. Would it be reasonable to cache pointers and check that Print/Sprint/etc don't visit the same pointer twice?

panic: reflect: call of reflect.Value.CanInterface on zero Value

Got it to panic:

panic: reflect: call of reflect.Value.CanInterface on zero Value [recovered]
	panic: reflect: call of reflect.Value.CanInterface on zero Value

goroutine 6 [running]:
panic(0x88f780, 0xc4200d4b00)
	/nix/store/1blrc2bxdfjrk3kl46l75dw4i1lpswp5-go-1.7.1/share/go/src/runtime/panic.go:500 +0x1ae
testing.tRunner.func1(0xc42009af00)
	/nix/store/1blrc2bxdfjrk3kl46l75dw4i1lpswp5-go-1.7.1/share/go/src/testing/testing.go:579 +0x474
panic(0x88f780, 0xc4200d4b00)
	/nix/store/1blrc2bxdfjrk3kl46l75dw4i1lpswp5-go-1.7.1/share/go/src/runtime/panic.go:458 +0x271
reflect.Value.CanInterface(0x0, 0x0, 0x0, 0xc41fff95a8)
	/nix/store/1blrc2bxdfjrk3kl46l75dw4i1lpswp5-go-1.7.1/share/go/src/reflect/value.go:896 +0xca
github.com/coreos/ignition/internal/vendor/github.com/kylelemons/godebug/pretty.(*Config).val2node(0xcd87e0, 0x0, 0x0, 0x0, 0x0, 0x420f58)
	/home/derek/go/src/github.com/coreos/ignition/gopath/src/github.com/coreos/ignition/internal/vendor/github.com/kylelemons/godebug/pretty/reflect.go:99 +0xe38
github.com/coreos/ignition/internal/vendor/github.com/kylelemons/godebug/pretty.(*Config).fprint(0xcd87e0, 0xc420026230, 0xc4200e3098, 0x1, 0x1)
	/home/derek/go/src/github.com/coreos/ignition/gopath/src/github.com/coreos/ignition/internal/vendor/github.com/kylelemons/godebug/pretty/public.go:61 +0xb8
github.com/coreos/ignition/internal/vendor/github.com/kylelemons/godebug/pretty.(*Config).Sprint(0xcd87e0, 0xc4200e3098, 0x1, 0x1, 0x0, 0x0)
	/home/derek/go/src/github.com/coreos/ignition/gopath/src/github.com/coreos/ignition/internal/vendor/github.com/kylelemons/godebug/pretty/public.go:83 +0x7a
github.com/coreos/ignition/internal/vendor/github.com/kylelemons/godebug/pretty.(*Config).Compare(0xcd87e0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
	/home/derek/go/src/github.com/coreos/ignition/gopath/src/github.com/coreos/ignition/internal/vendor/github.com/kylelemons/godebug/pretty/public.go:114 +0xcb
github.com/coreos/ignition/internal/vendor/github.com/kylelemons/godebug/pretty.Compare(0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
	/home/derek/go/src/github.com/coreos/ignition/gopath/src/github.com/coreos/ignition/internal/vendor/github.com/kylelemons/godebug/pretty/public.go:106 +0x77
github.com/coreos/ignition/internal/exec/stages/files.TestMapEntriesToFilesystems(0xc42009af00)
	/home/derek/go/src/github.com/coreos/ignition/gopath/src/github.com/coreos/ignition/internal/exec/stages/files/files_test.go:98 +0xed8
testing.tRunner(0xc42009af00, 0x9358b8)
	/nix/store/1blrc2bxdfjrk3kl46l75dw4i1lpswp5-go-1.7.1/share/go/src/testing/testing.go:610 +0xca
created by testing.(*T).Run
	/nix/store/1blrc2bxdfjrk3kl46l75dw4i1lpswp5-go-1.7.1/share/go/src/testing/testing.go:646 +0x530

To reproduce:

• clone https://github.com/dgonyeo/ignition
• git checkout panic-report
• ./test
• TestMapEntriesToFilesystems will trigger the panic in this library

Error calling String value method on nil

We were trying to use the PrintStringers option (it looks like PrintTextMarshalers would have the same issue), but we have some structs that include pointers to types with a String() method with a value receiver. time.Weekday provides a simple example where it detects that *time.Weekday implements Stringer and tries to call it with a nil pointer:

cfg := pretty.Config{
	PrintStringers: true,
}
cfg.Sprint(struct {
	Day *time.Weekday
}{
	Day: nil,
})

panic: value method time.Weekday.String called using nil *Weekday pointer

time.(*Weekday).String()
	<autogenerated>:45 +0xc1
github.com/kylelemons/godebug/pretty.(*Config).val2node()
	src/github.com/kylelemons/godebug/pretty/reflect.go:48 +0x1a6a
github.com/kylelemons/godebug/pretty.(*Config).val2node()
	src/github.com/kylelemons/godebug/pretty/reflect.go:95 +0xfa9
github.com/kylelemons/godebug/pretty.(*Config).fprint()
	src/github.com/kylelemons/godebug/pretty/public.go:98 +0x94
github.com/kylelemons/godebug/pretty.(*Config).Sprint()
	src/github.com/kylelemons/godebug/pretty/public.go:120 +0x6c

I may look into this a bit more, but wanted to go ahead and open an issue for it.

Special treatment for net.IP?

Hey!

net.IP is a bit annoying because:

1. it gets printed on several lines
2. it can uses two different representations

For the second point, net.ParseIP() will provide something like 0,0,0,0,0,0,0,0,0,0,255,255,192,0,2,12 (this is the IPv4-mapped IPv6 address). But one can also just get 192,0,2,12.

For me, it would be convenient if pretty could have a special case for IP by using its string representation. Of course, this would make an horrible special-case and maybe you don't want something like that.

Setting PrintStringers would defeat much of my use of the pretty printer since the string value is incomplete for most of my types. Maybe provide a list of types that should call String() if available?

Configuration for pretty.Compare

It would be nice if we can pass configurations to Compare, namely PrintStringers.

What I have in mind to have Compare method added to Config, so that
we can do something like the following:

config := &pretty.Config {
PrintStringers: true,
}
cmp := config.Compare(got, want)

Thanks!

Comparing two pointers should not nest

Hey!

When comparing two pointers, it seems that pretty.Compare() doesn't satisfy itself by the pointer values being the same. It tries to compare the content of those two pointers. Usually, this works, but sometimes, one pointer can change during iteration. I understand this may be difficult to fix as currently, both values are turned into strings, then compared as strings.