Skip to content

Commit

Permalink
Merge pull request #100 from crquan/patch-1
Browse files Browse the repository at this point in the history
make sure no trailing spaces
  • Loading branch information
sirupsen committed Jan 15, 2015
2 parents 3c5b048 + dcbe8d6 commit c6a969a
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 18 deletions.
34 changes: 16 additions & 18 deletions text_formatter.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,17 @@ func (f *TextFormatter) Format(entry *Entry) ([]byte, error) {
printColored(b, entry, keys)
} else {
if !f.DisableTimestamp {
f.appendKeyValue(b, "time", entry.Time.Format(time.RFC3339))
printKeyValue(b, "time", entry.Time.Format(time.RFC3339))
}
f.appendKeyValue(b, "level", entry.Level.String())
f.appendKeyValue(b, "msg", entry.Message)
printKeyValue(b, "level", entry.Level.String())
printKeyValue(b, "msg", entry.Message)
for _, key := range keys {
f.appendKeyValue(b, key, entry.Data[key])
printKeyValue(b, key, entry.Data[key])
}
}

b.WriteByte('\n')
return b.Bytes(), nil
return b.Bytes()[1:], nil
}

func printColored(b *bytes.Buffer, entry *Entry, keys []string) {
Expand All @@ -85,7 +85,7 @@ func printColored(b *bytes.Buffer, entry *Entry, keys []string) {

levelText := strings.ToUpper(entry.Level.String())[0:4]

fmt.Fprintf(b, "\x1b[%dm%s\x1b[0m[%04d] %-44s ", levelColor, levelText, miniTS(), entry.Message)
fmt.Fprintf(b, " \x1b[%dm%s\x1b[0m[%04d] %-44s", levelColor, levelText, miniTS(), entry.Message)
for _, k := range keys {
v := entry.Data[k]
fmt.Fprintf(b, " \x1b[%dm%s\x1b[0m=%v", levelColor, k, v)
Expand All @@ -104,21 +104,19 @@ func needsQuoting(text string) bool {
return true
}

func (f *TextFormatter) appendKeyValue(b *bytes.Buffer, key, value interface{}) {
func printKeyValue(b *bytes.Buffer, key, value interface{}) {
switch value.(type) {
case string:
if needsQuoting(value.(string)) {
fmt.Fprintf(b, "%v=%s ", key, value)
} else {
fmt.Fprintf(b, "%v=%q ", key, value)
}
break
case error:
if needsQuoting(value.(error).Error()) {
fmt.Fprintf(b, "%v=%s ", key, value)
} else {
fmt.Fprintf(b, "%v=%q ", key, value)
}
value = value.(error).Error()
default:
fmt.Fprintf(b, "%v=%v ", key, value)
fmt.Fprintf(b, " %v=%v", key, value)

This comment has been minimized.

Copy link
@crquan

crquan Jan 16, 2015

here really need a return, following code is a string.

}

if needsQuoting(value.(string)) {

This comment has been minimized.

Copy link
@stevvooe

stevvooe Jan 15, 2015

Collaborator

This kind of type conversion really needs a check. This PR is causing previously working code to crash on typed strings:

https://circleci.com/gh/docker/distribution/130

This comment has been minimized.

Copy link
@sirupsen

sirupsen Jan 15, 2015

Author Owner

Thanks, I'll revert it for now.

This comment has been minimized.

Copy link
@sirupsen

sirupsen Jan 15, 2015

Author Owner

Reverted and tagged 0.6.4

This comment has been minimized.

Copy link
@stevvooe

stevvooe Jan 15, 2015

Collaborator

Cheers. Let me know if you need anything to find a resolution.

This comment has been minimized.

Copy link
@sirupsen

sirupsen Jan 15, 2015

Author Owner

\cc @crquan

This comment has been minimized.

Copy link
@sirupsen

sirupsen Jan 15, 2015

Author Owner

Fix is fairly easy, but this formatter is getting out of hand in complexity. I need to clean it up.

This comment has been minimized.

Copy link
@crquan

crquan Jan 16, 2015

sigh, sorry just see you reverted, but the fix is easy, there need a return in default case of switch, the same 3 commits with the return in the same branch now, let me know if you want another pr @sirupsen
https://github.com/crquan/logrus/commits/patch-1

fmt.Fprintf(b, " %v=%s", key, value)
} else {
fmt.Fprintf(b, " %v=%q", key, value)
}
}
27 changes: 27 additions & 0 deletions text_formatter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,30 @@ func TestQuoting(t *testing.T) {
checkQuoting(false, errors.New("invalid"))
checkQuoting(true, errors.New("invalid argument"))
}

func TestTextPrint(t *testing.T) {
tf := &TextFormatter{DisableColors: true}
byts, _ := tf.Format(&Entry{Message: "msg content"})

// make sure no leading or trailing spaces
if string(byts) !=
"time=\"0001-01-01T00:00:00Z\" level=panic msg=\"msg content\"\n" {
t.Errorf("not expected: %q", string(byts))
}
}

func TestColorPrint(t *testing.T) {
tf := &TextFormatter{ForceColors: true}
entry := WithField("testkey", "value")
entry.Message = "msg content"
byts, _ := tf.Format(entry)

// make sure no leading or trailing spaces
if string(byts) !=
"\x1b[31mPANI\x1b[0m[0000] " +
// length 44 plus one space
"msg content " +
"\x1b[31mtestkey\x1b[0m=value\n" {
t.Errorf("not expected: %q", string(byts))
}
}

0 comments on commit c6a969a

Please sign in to comment.