Support --colour auto

[Morganamilo]

For the sake of consistency I would like Yay to support the same colour flags as pacman --color always|auto|never. Currently Yay reads the colour setting from pacman's config but is totally unaffected by the flags.

Implementing always and never is pretty straightforward. auto on the other hand checks if we are outputting to a terminal and only enables colour if we are.

Implementing this is a little more difficult. The best way I can see to do that is to pull in a new dependency: http://golang.org/x/crypto/ssh/terminal. It's not part of the standard library but it is at least from golang.org.

Then again a whole dependency for a tiny feature? It does kind of seem like a waste. I know we're trying to keep dependencies to a minimum.

Implementing this ourselves would require a system call which I would rather stay away from.

We could just implement it 'incorrectly' and have auto to be the same as never or the same as always.

What do you think?

[Jguer]

Pulling a new dependency makes me sad. I would skip auto (or make it always never), for the sake of keeping yay less bloated.

Playing with syscalls seems a tad overkill for something as simple as colour which is already "enough" supported.

[Morganamilo]

Gotcha auto=never when I get round to it.

[nairaner]

Another easy way would me to just check if TERM environment variable contains "color". This should work in most cases.

[Morganamilo]

That will not work. the TERM environment variable does not change if redirected to a file.

morganamilo@Vinyl numbermenu ~/git/yay % echo $TERM > a
morganamilo@Vinyl numbermenu ~/git/yay % cat a         
xterm-256color

[aswild]

It may be trickier to figure out whether a specific terminal supports color, but it should be easy to detect whether stdout is a TTY. In C, the isatty(3) function provides this.

I found a Go example which uses syscall in go-isatty.
It's a one-liner using only the standard library, so no need to add a dependency, just adapt/copy that function somewhere in yay. go-isatty is MIT licensed so no worries there.

[Morganamilo]

As I mentioned in my initial post I did consider the syscall option. My problems with a syscall is that I don't really think high level applications should be making syscalls, that's the standard library's job. Also I don't really want to be importing unsafe outside of go-alpm.

Another thing is that syscalls are OS specific. Pacman is OS agnostic so I think Yay should at least try to be too. Not that I have tried Yay on another OS or really expect much of an user base there, I have no idea what the user numbers are for projects like Arch Hurd and pacBSD.

http://golang.org/x/crypto/ssh/terminal provides implementations for BSD and a couple others at least. But doing all that just for colour settings seems like a bit much.

[aswild]

I don't really think high level applications should be making syscalls

That's a philosophical argument rather than a practical one. Why would the syscall package exist at all if not for letting applications use features which aren't exposed by the standard library? Passing arguments to syscalls is one of the explicit examples for using unsafe.

Pacman is OS agnostic so I think Yay should at least try to be too

This is fair, but doesn't mean Yay can only support the lowest common denominator. Since (I assume) the vast majority of Yay usage is on Arch Linux, why not support a useful auto for Linux, and hard-code auto=never for other OSes?

At a minimum, can the Pacman color setting be mentioned in the Yay manpage? I had to get frustrated and confused enough to go googling and reading stuff on Github to figure out why I sometimes got color from Yay and sometimes didn't on different machines.

[Morganamilo]

At a minimum, can the Pacman color setting be mentioned in the Yay manpage? I had to get frustrated and confused enough to go googling and reading stuff on Github to figure out why I sometimes got color from Yay and sometimes didn't on different machines.

Can we take a step back and focus on this then? This issue was just for dealing with the --color auto flag nothing else. At some point Yay used to read a color boolean from its own config, now it reads the color value from pacman.conf. Maybe the change in behaviour is what caused the problem?

[Morganamilo]

To add though I do agree with you on adding some extra info to the man page. Although not specifically about color settings but more of a general "yay will inherit settings from pacman.conf".

[Morganamilo]

Thinking about it because we read the colour value through pacman.conf we could do the isatty() inside off go-alpm maybe embed some c to do it so it remains cross platform. Although as I have mentioned before, the pacman.conf parsing in go-alpm is a little sketchy. It stores the colour option as a bit field which means it can only have two options. Refactoring that would probably take a bit of effort.

[aswild]

Maybe the change in behaviour is what caused the problem?

I wasn't aware, so that could be it. Was that change relatively recent? Here was my experience as a user:

• I enabled color in pacman.conf on one of my computers a while ago, then forgot that was even a thing and never did it on my other Arch installs.
• After a Yay update, color suddenly stopped working most of the time
• I searched yay --help, the man page, and the Yay config file and found nothing mentioning color
• I wrote it off as something weird in my ssh or terminal setup breaking auto-detection until today when I got fed up and actually googled the problem, which led me here

If expanding the man page, I would recomend including the word "color" at least somewhere so that it can be searched for. Maybe like "yay will inherit settings from pacman.conf (e.g. color, xxx, yyy)"

[aswild]

I also like the idea of embedding isatty() in some C rather than using the syscall directly. Gives some free cross-platform compatibility and wouldn't require an unsafe pointer. (though then it raises questions of whether it should be a Yay setting, read from pacman, or some combination of both if Yay is unconfigured)

[Morganamilo]

It was this pr that did it #113. I would have expected everyone wanting Yay to be in colour would already have Pacman colour enabled I did not expect the confusion it created. It was also added to the readme:

Frequently Asked Questions

• Yay does not display colored output. How do I fix it?
  Make sure you have the Color option in your /etc/pacman.conf v2.297 lost its colors #123

But yes mentioning it in the man page is a thing that should be done.

though then it raises questions of whether it should be a Yay setting, read from pacman, or some combination of both if Yay is unconfigured

It's pretty set in stone that:

• Yay will read the colour setting from pacman.conf
• --color will override the config setting.

Everything discussed about this is an implementation problem, the behaviour apart from auto Is already decided.

[aswild]

Sounds good to me. I opened a PR for a man page comment if you're happy with that wording. (edit: looks like you were since it got merged while I was typing out this comment)

For isatty(), I found it can be as simple as

// #include <unistd.h>
import "C"

and then checking C.isatty(1) != 0. (You probably already knew that, but I'm new to Go and like trying things out)

Thanks for making Yay a great tool, I'm definitely happy having switched from yaourt.

[Morganamilo]

Yeah it looked good, should probably add to it a bit though to include info on vcs.json and the aur package list cache but this is fine for now.

With the c stuff you'll probably have to import unsafe as well that's why I would like to have this implemented in go-alpm somehow because we already use a bunch of c and unsafe stuff. I'd rather keep that stuff out of Yay if I can.

[aswild]

I don't think unsafe is required to use C, unless cgo automatically does that under the hood. Ultimately, isn't the result the same regardless of whether the C calls are in go-alpm or Yay since all the Go code gets linked into the main executable?

IMHO an isatty wrapper wouldn't make much sense in go-alpm because it's not really related to libalpm, but I won't argue about how you and @Jguer want to architect your project.

$ cat isatty.go
package main

// #include <unistd.h>
import "C"
import "fmt"

func main() {
    if C.isatty(1) != 0 {
        fmt.Println("output to TTY")
    } else {
        fmt.Println("output to pipe")
    }
}

$ go build isatty.go

$ ldd ./isatty
        linux-vdso.so.1 (0x00007fff69925000)
        libpthread.so.0 => /usr/lib/libpthread.so.0 (0x00007f9649449000)
        libc.so.6 => /usr/lib/libc.so.6 (0x00007f9649092000)
        /lib64/ld-linux-x86-64.so.2 => /usr/lib64/ld-linux-x86-64.so.2 (0x00007f9649667000)

$ ./isatty
output to TTY

$ ./isatty | cat
output to pipe

The glibc implementations of isatty and the underlying tcgetattr only use stack variables/pointers which Go would never touch, so that doesn't seem "unsafe" to me.

[Morganamilo]

go-alpm also handles the pacman config parsing even though it doesn't really have anything to do with libalpm. So if we added it to go-alpm it would be part of that.

I don't know how to feel about it mainly because it feels wrong to include a whole other language just for the tiniest feature. I get that's not exactly a great argument.

I'd probably prefer http://golang.org/x/crypto/ssh/terminal over c but I would probably give the OK for either if @Jguer does too.

[aswild]

Fair point about the pacman config parsing.

My take is that wrapping isatty isn't really adding a whole other language since C is already used in go-alpm. If yay really was pure Go, that would be a different story.

Using golang.org/x/crypto/ssh/terminal (which also pulls in golang.org/x/sys/unix) would be a lot of extra overhead just for the IsTerminal function, but it might be worthwhile if Yay would use more of its features like abstracting out the color escape codes (though it looks like vt100 is all that's supported anyway)

[Morganamilo]

I added a more fleshed out FILES section over at #230 now that I'm done with other stuff. Feel free to check it out and tell me what you think.

[AndydeCleyre]

The pacman.conf man page describes the Color option to mean

Automatically enable colors only when pacman’s output is on a tty.

It's unexpected for yay to read that option and interpret it differently. It also leads to unexpected behavior when running, say:

yay -Si musl pacman yay | grep "^URL"

URL             : http://www.archlinux.org/pacman/
URL             : http://www.musl-libc.org/

[squalou]

Sorry to jump aboard the discussion for a detail : I'm more interested in the --color=never option (in order to script things more easily).
Is it something that will come with the resolution of this issue, or something else entirrely ?

thx !

[Morganamilo]

--color never already works. color=never does not, I just realised I got it the wrong way round, = should only work on long opts but I made it only work on short opts.

[squalou]

thx ! didn't even try to tell the truth. Didn't see it in the --help so I didn't even try any further.