-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add multiline option to Pretty.ColorizeJSON #484
Conversation
utils/pretty/pretty.go
Outdated
if p.noColor { | ||
return data | ||
} | ||
|
||
f := prettyjson.NewFormatter() | ||
f.Indent = 0 | ||
f.Newline = "" | ||
if multiline == false { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
!multiline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated but I really don't like !
. It's too small. I prefer explicit false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's go way:
➜ a git:(dev) ✗ gometalinter .
main.go:7:5:warning: should omit comparison to bool constant, can be simplified to !a (S1002) (megacheck)
Also go source code:
➜ src gogrep 'if !' |wc -l
8661
➜ src gogrep 'if.*== false' |wc -l
17
➜ src
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like the false == multiline
it's more "conventional but we never do that anywhere in the code. I think it's better to have consistency. Or if you want just change the parameter with inline
like that you have a if inline
;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok guys, I give up on this ;)
Very simple PR to add the multiline option to Pretty.ColorizeJSON function.
It is useful for a following PR.