fatih/color

feature request: Add Color merge functionality

apexskier opened this issue ยท 9 comments

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

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

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

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

Go for it

bl-ue commented

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

bl-ue commented

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

@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.

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

fatih commented

Closing this based on the comment here: #129 (comment)