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

Println variants have inconsistent spacing #25

Closed
aarondl opened this issue May 3, 2014 · 4 comments · Fixed by #26
Closed

Println variants have inconsistent spacing #25

aarondl opened this issue May 3, 2014 · 4 comments · Fixed by #26

Comments

@aarondl
Copy link
Contributor

aarondl commented May 3, 2014

The Go fmt package has different spacing between arguments for Print and Println. This is described in the documentation.

Logrus does not use the same spacing as the go fmt/log packages inhibiting it's ability to be a drop in replacement. Can this be changed or is forking necessary here?

@sirupsen
Copy link
Owner

sirupsen commented May 3, 2014

Good catch @aarondl. This is definitely a bug, I'll look into it.

@sirupsen
Copy link
Owner

sirupsen commented May 3, 2014

This can be changed for sure. I originally did this instead of just calling fmt.Sprintln because I didn't want a newline at the end, and I thought that's all it did. Newlines don't make sense when you're treating the message as a value.

What do you think about having the same concatenation rules, but still leaving out the newline?

@aarondl
Copy link
Contributor Author

aarondl commented May 3, 2014

Of course, the newlines are not what we want in a logging package. You'll notice Go's log package already has this behavior: http://golang.org/src/pkg/log/log.go#L124

So what you've mentioned is perfect :D

@sirupsen
Copy link
Owner

sirupsen commented May 3, 2014

Alright, fixed in #26. Let me know if everything looks good to you :)

sirupsen added a commit that referenced this issue May 4, 2014
entry: log with spaces between all operands on *ln #25
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 a pull request may close this issue.

2 participants