issue with tabs when using prioritization

[gammons]

I can't seem to figure this one out. It only happens sometimes.

[vokiel]

I noticed that without color.Italic https://github.com/gammons/todolist/blob/master/todolist/formatter.go#L49 it works fine.

Also it works with color.Italic but with different format string " %s \t%s\t%s\t%s\t\n" (additional space after first %s) https://github.com/gammons/todolist/blob/master/todolist/formatter.go#L51

[gammons]

@vokiel oh nice. Do you want to submit a PR for that?

[vokiel]

It depends on the desired output. For me bold is enough, others may want both (bold + italic).

In the second, extra space could be fine. Or maybe it would be better to do more digging into formatter and/or http://github.com/fatih/color (what exactly Italic adds to the output). I would also check if concatenating strings work better in this case.

[mikezter]

Hey @vokiel, did you manage to reproduce the error consistently?

[vokiel]

Hi @mikezter, I've did a simple check on sample based on the todolist:

package main

import (
        "fmt"
        "text/tabwriter"
        "github.com/fatih/color"
        "strconv"
        "os"
)

type Formatter struct {
        Writer       *tabwriter.Writer
}

func main() {
        w := new(tabwriter.Writer)
        w.Init(os.Stdout, 0, 8, 0, '\t', 0)
        f := &Formatter{Writer: w}

        yellow := color.New(color.FgYellow)
        yellow.Add(color.Italic)
        yellow.Add(color.Bold)

        fmt.Fprintf(f.Writer, " %s\t%s\t%s\n", yellow.SprintFunc()(strconv.Itoa(43)), "[ ]","Just a simple text")
        f.Writer.Flush()
}

Results:

root@f0a9f6131ae8:/usr/src/myapp# go run app.go
 43[ ]  Just a simple text

With yellow.Add(color.Italic) commented-out

root@f0a9f6131ae8:/usr/src/myapp# go run app.go
 43     [ ]     Just a simple text

[ingorichter]

it looks like yellow.Add(...) swallows the tab character. What are the control characters inserted by yellow.Add(color.Italic)?

[mikezter]

I think the relevant code of package color is here https://github.com/fatih/color/blob/master/color.go#L365 and here https://github.com/fatih/color/blob/master/color.go#L346

constants are defined here: https://github.com/fatih/color/blob/master/color.go#L45

I am not too familiar with ANSI escapes (or ist SGR?) so i'm interested to learn about the problem here.

[ingorichter]

I modified the test program from @vokiel and instead of

fmt.Fprintf(f.Writer, " %s\t%s\t%s\n", yellow.SprintFunc()(strconv.Itoa(43)), "[ ]","Just a simple text")

I printed the string directly to the console with

var t = fmt.Sprintf(" %s\t%s\t%s\n", yellow.SprintFunc()(strconv.Itoa(43)), "[ ]","Just a simple text")
fmt.Print(t)

The result is
43 [ ] Just a simple text (this looks alright)

It looks like tabwriter is the culprit

[ingorichter]

Update:
I was able to isolate the issue and it repro it with the least amount of data:
Save the following json as .todos.json and run todolist l and you will see it.

[{
    "id": 10,
    "subject": "Task 1",
    "projects": [],
    "contexts": [],
    "due": "",
    "completed": false,
    "completedDate": "",
    "archived": false,
    "isPriority": true
}]

The issue is that the id is double digit. With single digits this issue doesn't happen at all.

The test data in the repo doesn't have this combination of double digit and isPriority

[NonerKao]

Any updates? I found this, which states that color and tabwriter are not compatible to each other.

[ingorichter]

@NonerKao that was also my observation. The tabwriter is confused by the additional control characters. I was looking into changing the tabwriter to add support for those control sequences. I'm currently traveling and don't have support to my machine, but when I'm back in 8 days I'd like to setup a PR with my changes.

[NonerKao]

@ingorichter That's great! Looking forward to the fix :)

Just FYI, the text/tabwriter is official and The text/tabwriter package is frozen and is not accepting new features. (text/tabwriter/tabwriter.go:11)
So maybe fixing fatih/color would be another choice? Maybe something like this patch at fatih/color would be helpful.

[NonerKao]

Any news?

[gammons]

I ran some tests using @prataprc 's patch to color but it seems to still have not fixed the issue.

[NonerKao]

Because the root cause lies in tabwriter and its algorithm.

I will deal with this recently.

[NonerKao]

I've finished the color-aware tabwriter modification, so I will submit a PR later which will replace the dependency text/tabwriter with github.com/NonerKao/color-aware-tabwriter .

[ingorichter]

Okay, now I'm way behind of the things I wanted to do after my long trip. Thanks for putting up the fix. I'll check it out in the afternoon. 👍

[NonerKao]

@ingorichter Thanks for the survey! What you pointed out was pretty helpful for me to submit this patch.