Add (optional) color to make output of --help easier to read

[cleichner]

No description provided.

[chriscool]

Optional probably means a config flag (like color.ui in Git).

And generaly speaking, colors in the output should be removed when the output is not a tty, otherwise it can create problems when output is piped into a command.

As --help would probably be piped to a pager, we could avoid checking if the output is a tty and instead tell people to use a pager that can accept colors (like less -R). Or we could launch the pager ourselves like Git sometimes does...

Also the output of --help could be quite nice already if it was really a man page, or if there was a flag (like git help -w) to open an HTML version of the help text in a web browser.

[chriscool]

https://github.com/mattn/go-isatty/ can be used to know if the output is on a terminal.

[chriscool]

See also issue #275 about man pages and issue #303 about implementing the ipfs help command.

[dborzov]

@chriscool go-isatty looks great, but maybe going throuth the trouble of vendoring one more package that just wraps one system call is a little too much for our purposes?

Checking if the stdout is on a terminal goes through this system call for stdoutput tty parameters on both linux and BSD (Mac OS):

func isTerminal() {
    const stdoutFD = 1
    var dimensions [4]uint16

    if _, _, err := syscall.Syscall6(syscall.SYS_IOCTL, uintptr(stdoutFD), uintptr(syscall.TIOCGWINSZ), uintptr(unsafe.Pointer(&dimensions)), 0, 0, 0); err != 0 {
        return 'Not a terminal'
    }
    terminalWidth = int(dimensions[1])
       return "Is a terminal with " + string(terminalWidth) + " symbol width";
}

Only on Windows it is different, but Powershell does not understand the standard escape symbol coloring anyway, if I understand it correctly.

I have been using this system call in my lsp project: dborzov/lsp@345d5ff#diff-ec233a1b052bc886bbdad1626a6443b6R34

and it also yeilds other useful parameters like dimensions of the terminal (symbol width) which you can use to format nice things (like hr lines).

I am going to work on this issue, if everyone is cool with this approach of just using that system call for Unix systems.

[whyrusleeping]

@dborzov I like this, vendoring is great, but an entire package for a single call is a bit much. On windows i think we can just always return false (windows doesnt have ttys!!) and not have colors in windows.

[btc]

To be fair, there are only three functions in the package. (bsd, linux, windows)

func IsTerminal(fd uintptr) bool {
    var st uint32
    r, _, e := syscall.Syscall(procGetConsoleMode.Addr(), 2, fd, uintptr(unsafe.Pointer(&st)), 0)
    return r != 0 && e == 0
}
func IsTerminal(fd uintptr) bool {
    var termios syscall.Termios
    _, _, err := syscall.Syscall6(syscall.SYS_IOCTL, fd, ioctlReadTermios, uintptr(unsafe.Pointer(&termios)), 0, 0, 0)
    return err == 0
}
func IsTerminal(fd uintptr) bool {
    var termios syscall.Termios
    _, _, err := syscall.Syscall6(syscall.SYS_IOCTL, fd, ioctlReadTermios, uintptr(unsafe.Pointer(&termios)), 0, 0, 0)
    return err == 0
}

[whyrusleeping]

haha, just that? okay then

[jbenet]

maybe going throuth the trouble of vendoring one more package that just wraps one system call is a little too much for our purposes?

vendoring is great, but an entire package for a single call is a bit much.

This is one of the really stupid things about how Go packaging is done today. That current vendoring overhead encourages this kind of thinking. to those of us that have had the lovely experience of working with npm and proper packaging, packages tend to be only one function.

Whenever possible, reuse code. vendoring is not a huge deal. I know godep is not the nicest/easiest thing in the world, but using packages is TRTTD.

[dborzov]

@jbenet , yeah, fair enough, we should vendor this out.

[dborzov]

Plus, it turned out there is already an isTerminal() check function in the source written by @mappum ! https://github.com/jbenet/go-ipfs/blame/master/commands/cli/parse.go#L345 Pretty neat :)

[daviddias]

@jbenet are you still up to CR @dborzov's PR (#377)? We will need to have it rebased by now though, @dborzov would you have availability to do it?

[brodo]

I've started working on this (see screenshot).
Currently, I'm checking if the output is a TTY with the isTty function in cli/parse.go. The problem is checking the config. Currently the config is only read on demand. If we want to have the option in the config, it needs to be read it directly in ipfs/main.go. My personal preference would be not to put the option in the config file, but introduce another command line flag like --color.

If you guys still prefer the config file, I need some help in how to restructure the starting process.

[hsanjuan]

I like --color, but let's see if others can weight in. Maybe there are considerations for having this in the config.

[btc]

If we want to have the option in the config, it needs to be read it directly in ipfs/main.go.

Perhaps, when no config exists, the program can be treated as though the color option is not specified.