fatih / color

Color package for Go (golang)
https://pkg.go.dev/github.com/fatih/color
MIT License
7.24k stars 615 forks source link

feature request: Add Color merge functionality #128

Closed apexskier closed 3 years ago

apexskier commented 3 years ago

I have two color objects that I'd like to "merge" together. A quick example would be:

func foo(a color.Color, b color.Color) {
    a.Merge(b).Println("hello world")
}

func main() {
    color1 := color.New(color.BgRed)
    color2 := color.New(color.FgGreen)

    modifier1 := color.New(color.Underline)
    modifier2 := color.New(color.Bold)

    foo(color1, modifier1)
    foo(color1, modifier2)
    foo(color2, modifier1)
    foo(color2, modifier2)
}

This could follow the same semantics as Color.Add in terms of overwriting non-compatible attributes.

Alternatively, exposing the current attributes of a color object would allow me to do a.Add(b.Attributes...) to achieve the same effect.

If this seems reasonable I can submit a PR

bl-ue commented 3 years ago

Probably as simple as append(a.params, b.params).

apexskier commented 3 years ago

Yeah, internally I think this would be as simple as this (based on the current Add implementation)

func (c *Color) Merge(others ...*Color) *Color {
    for _, other := range others {
        c.params = append(c.params, other.params...)
    }
    return c
}
bl-ue commented 3 years ago

@apexskier If you don't mind I'll submit a PR.

apexskier commented 3 years ago

Go for it

bl-ue commented 3 years ago

@apexskier Are you thinking for it to modify the original Color, or return a new one?

bl-ue commented 3 years ago

It could also be like this:

a := color.New(color.FgRed)
b := color.New(color.Bold, color.BgGreen)
c := color.New(color.Italic)
m := color.Merge(a, b, c /* ... */)        // returns a new color without modifying a, b, or c

But that isn't like the other functions because most of them are methods with a receiver of type *Color.

@apexskier What do you think?

bl-ue commented 3 years ago

@apexskier I've got it done now. Just need to confirm which path to go, modify receiver or return a new Color object. I personally like the idea better of returning a new color object with colors.Merge.

apexskier commented 3 years ago

I like the more immutable pattern you're suggesting. That'll work better for my use case.

fatih commented 3 years ago

Closing this based on the comment here: https://github.com/fatih/color/pull/129#issuecomment-770465089