Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compare options are not optional #8

Open
anyasabo opened this issue Oct 1, 2020 · 2 comments
Open

Compare options are not optional #8

anyasabo opened this issue Oct 1, 2020 · 2 comments

Comments

@anyasabo
Copy link

anyasabo commented Oct 1, 2020

Trying to compare identical empty objects or other simple objects results in a panic if no options are provided. The name is a little misleading since options imply (to me at least) that they are not required.

For instance:
diff, _ := jsondiff.Compare([]byte({}), []byte({}), nil)

Adding opts := DefaultConsoleOptions() and passing it in (as in the unit tests) prevent it from panicking. I didn't get a chance to see what it is expecting just yet.

Output:

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x5b272a]

goroutine 6 [running]:
testing.tRunner.func1.1(0x5d7ce0, 0x760ed0)
	/usr/local/go-faketime/src/testing/testing.go:1076 +0x30d
testing.tRunner.func1(0xc000001b00)
	/usr/local/go-faketime/src/testing/testing.go:1079 +0x41a
panic(0x5d7ce0, 0x760ed0)
	/usr/local/go-faketime/src/runtime/panic.go:969 +0x175
github.com/nsf/jsondiff.(*context).printDiff(0xc000045ec0, 0x5d6c60, 0xc000073260, 0x5d6c60, 0xc000073290)
	/tmp/gopath249660858/pkg/mod/github.com/nsf/jsondiff@v0.0.0-20200515183724-f29ed568f4ce/jsondiff.go:329 +0x4ea
github.com/nsf/jsondiff.Compare(0xc00002c9b0, 0x8, 0x8, 0xc00002c9b8, 0x8, 0x8, 0x0, 0x4b5a86, 0x4af9f070, 0x0)
	/tmp/gopath249660858/pkg/mod/github.com/nsf/jsondiff@v0.0.0-20200515183724-f29ed568f4ce/jsondiff.go:414 +0x296
main.Test_jsondiffnoopts(0xc000001b00)
	/tmp/sandbox175228423/prog.go:11 +0xaa
testing.tRunner(0xc000001b00, 0x61efb8)
	/usr/local/go-faketime/src/testing/testing.go:1127 +0xef
created by testing.(*T).Run
	/usr/local/go-faketime/src/testing/testing.go:1178 +0x386

Simple examples here: https://play.golang.org/p/vSdHmaOY3zw

On a different note, thank you for sharing this repo. I have a project where we are not using it for debugging, but it is useful for finding superset matches in arbitrary json bodies.

@pjebs
Copy link

pjebs commented Mar 13, 2021

Yes. You can pass in any option and just ignore the second return value of Compare function.

In terms of API design, the different built-in options should return pointers instead of concrete types.

@nsf
Copy link
Owner

nsf commented Sep 18, 2021

Sorry for a late response. I guess calling "options" is not the best idea. As it's not optional. I will think about renaming it to "Arguments" or something like that before releasing 1.0.

Sadly I don't see a way to actually make it optional as there is no generic format. Most formats are specific to their environment. Be it JSON, linux terminal or HTML.

Perhaps I should call it "Formatt". E.g. "DefaultConsoleFormat()". But then it contains options also right now. Naming is hard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants