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

use log for errors, handle invalid flags and explicitly print to stdout #6

Closed
wants to merge 1 commit into from

Conversation

aybabtme
Copy link

@aybabtme aybabtme commented Mar 9, 2017

Hey there! Sorry for the random unsolicited PR.

While exploring this tool, I naively ran go-outline without any arguments and saw:

$ go-outline
error: Could not parse file
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x2263]

goroutine 1 [running]:
panic(0x110ee0, 0xc42000a0c0)
	/Users/antoine/go/src/runtime/panic.go:500 +0x1a1
main.main()
	/Users/antoine/gocode/src/github.com/lukehoban/go-outline/main.go:53 +0x223

That got me thinking "Hm I feel this shouldn't panic, and if it can't parse the file, why does it continue trying to work".

  1. So I started by replacing the "Could not parse file" message with a log.Fatal call.
  2. Then I setuplog to have the prefix of the app, so that when run from scripts/tools, it's clear what tool is generating the error message.
  3. Then I changed the in-loop-error-messages to be using the log package too, but in a "do not explode, carry on" manner.
  4. Then I thought: well really the flags could be validated with more specific errors, and when invalid calls are made, we could print the "usage" block like usual CLI tools do.
  5. Then I had some extra moment of lucidity and replaced the 2 lines json.Marshal thing with a single line json.Encode call over stdout.

Now calling this tool without any flag does:

$ go-outline
go-outline: invalid usage: no source or file provided
  -f string
    	the path to the file to outline
  -imports-only
    	parse imports only
  -src string
    	source code of the file to outline

Hope all this is ok with you!

r: @lukehoban

@aybabtme
Copy link
Author

aybabtme commented Mar 9, 2017

Just saw #5, which sort of partially address the same thing. However, I think this PR (mine) is more comprehensively improving things.

@aybabtme aybabtme closed this Feb 7, 2021
@aybabtme aybabtme deleted the friendly-error-handling branch February 7, 2021 08:07
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

Successfully merging this pull request may close these issues.

1 participant