[lrstanley]

Changes

• Initial implementation of border blending (gradients).
  • New Style.BorderForegroundBlend(colors...) method for storing the palette the user wants to use for blending.
  • Current blended border by default always starts from the same position (top left -> top right, -> bottom right -> bottom left -> top left), except when using Style.BorderForegroundBlendOffset(int) (see below).
  • New Style.BorderForegroundBlendOffset(int) method for controlling the offset where the gradient begins on the border.
    • See below examples for a rotating/spinning border which utilizes this.
  • Associated Get<X>() and Unset<X>() methods for the above 3 methods.

Performance notes

• Naturally, blended borders have worse performance than regular borders, especially with the current way borders are rendered. I tried to keep the current implementation mostly the same for non-blended borders, though there is a small perf loss.
• I made some minor tweaks to other methods in lipgloss to help reduce some of the performance impact, and other simple things technically not related to borders (e.g. Width() should be 30-50% less CPU usage, lot of allocs -> 0 allocs).

Checklist before requesting a review

• I have read CONTRIBUTING.md
• I have performed a self-review of my code

Screenshots

More screenshots and example code

Border on all sides

lipgloss.NewStyle().
		Border(lipgloss.RoundedBorder()).
		BorderForegroundBlend(
			charmtone.Cherry,
			charmtone.Charple,
			charmtone.Guac,
			charmtone.Sriracha,
		).
		Width(50).
		Height(8).
		Render("Hello, world!")

Border on bottom only

see notes below about "still internally generate a full blend of the full size"

lipgloss.NewStyle().
		Border(lipgloss.RoundedBorder(), false, false, true, false).
		BorderForegroundBlend(
			charmtone.Cherry,
			charmtone.Charple,
			charmtone.Guac,
			charmtone.Sriracha,
		).
		Width(50).
		Height(2).
		Render("Hello, world!")

Misc border types

double border:

normal border:

outer half block border:

ascii border:

Rotating/spinning border

Definitely increases CPU usage due to the FPS, but could be cool for temporary states in an application (e.g. for X seconds after typing, it updates the keeps rotating the border, and stops once the user stops typing or is inactive). The below example renders at 15FPS @ 5 cell advances per frame.

or blending the border with the background color (modifying the below code a little bit):

Code for this example can be found within examples/blending/border-blend-rotation

---

Reviewer Notes

• Only foreground blending is supported. I suspect it would be extremely unlikely that users would want background colors blended, but we could always add this in the future if there is a strong desire.
• Will still only render the specific border sides depending on what is enabled, but the blending is agnostic to it. As in, it will still internally generate a full blend of the full size, but only apply the colors to the visible borders.
• Ideally, it would be nice to "cache" the generated blend to reduce compute on render, but I couldn't think of a simple way of doing this natively.
  • If you generate the blend during set() on the style, it won't correctly scale if the size fields change.
  • In one of my projects, I use an in-memory LFU cache based on the size, but that only works well when the palette/wrapping/etc is static.

[meowgorithm]

This PR is exceptional and we really appreciate the work you put into both the code and documentation. Thank you.

Two thoughts on API:

1. In terms of API, what if we simply changed BorderForeground to be variadic so it can auto-upgrade to a blend?
2. Does it make sense to group border options together in a functional argument kind of way? I don't think so, but throwing it out there as an option.

Here’s what the would API look like in both cases:

lipgloss.NewStyle().
	Border(lipgloss.RoundedBorder()).
	BorderForeground(
		charmtone.Charple,
		charmtone.Guac,
		charmtone.Cherry,
	).
	BorderOptions(
		lipgloss.WithBorderBlendWrap(),
		lipgloss.WithBorderBlendOffset(4),
	).
	Width(50).
	Height(8).
	Render("Hello, world!")

Also if you want to take a deeper pass on perf around borders after we merge this, go for it. In general the renderer has changed very little since I first put it together a few years ago and there’s definitely room for improvement.

[lrstanley]

In terms of API, what if we simply changed BorderForeground to be variadic so it can auto-upgrade to a blend?

Not opposed, though the only problem with variadic is that there is a potential that someone might provide 0 arguments. I assume in this situation, it's fine to have it be a no-op. As far as properties are concerned, we should probably still store them behind the scenes as separate properties to keep memory usage lower in the more common path.

EDIT: other gotcha I just noticed, what about some of the GetBorder<X>() methods? Those currently only return specific sides. Adding a GetBorderForeground() to match the setter, feels weird because there are multiple potential things it could return.

Does it make sense to group border options together in a functional argument kind of way? I don't think so, but throwing it out there as an option.

Given there wouldn't really be any other options that are structured in that way, will likely make the API feel a bit more weird. Especially because if we did, I'd think all other border options should also use a similar API, further segmenting the style API.

[meowgorithm]

All good points.

In terms of API design, I'm hoping to avoid ambiguity when Style.BorderForeground and Style.BorderBlendForeground are set. I'd love for users not to have wonder which takes prescience, whether one unsets the other, or why a blend is present on the foreground after unsetting a foreground.

One thought is that we could introduce a new type for border blends:

border := lipgloss.NewBorderBlend(
	charmtone.Charple,
	charmtone.Guac,
	charmtone.Cherry,
).Wrap(true).Offset(4)

lipgloss.NewStyle().
	Border(lipgloss.RoundedBorder()).
	BorderForeground(border)

If we don't go with something like this, what's the prescidence between Style.BorderForeground and Style.BorderBlendForeground?

[lrstanley]

border := lipgloss.NewBorderBlend(
	charmtone.Charple,
	charmtone.Guac,
	charmtone.Cherry,
).Wrap(true).Offset(4)

lipgloss.NewStyle().
	Border(lipgloss.RoundedBorder()).
	BorderForeground(border)

Is it expected that this new type would implement the color.Color interface (and then be cast behind the scenes to the correct type to get out the necessary info), so that it can be passed in as-is?

• If not, I can't envision how both non-blend and blend could be passed into that method, since that's the contract for normal colors being passed in. Same with getting the data back out with GetBorderForeground().
  • I suspect long-term, Go will add method-based generics, which could help.
• If yes, this creates a bunch of new problems, e.g., that the same arguments into BorderForeground() can't be passed into Border<side>Foreground() methods (since this doesn't support 1 side being blended, but others being regular colors) despite the type of the argument supporting it.

I'd love for users not to have wonder which takes prescience, whether one unsets the other, or why a blend is present on the foreground after unsetting a foreground.
[...]
If we don't go with something like this, what's the prescidence between Style.BorderForeground and Style.BorderBlendForeground?

Trying to put myself as a regular user, adding blending, it makes sense that adding blending effectively "takes over" the coloring, and with the appropriate warnings in the comment of the method (could also add to the BorderForeground() and Border<side>Foreground() methods so it's even more clear), I would think it would be sufficient. I think even with custom types, the problem of prescience with ordering/priority still doesn't go away (just reduced a little bit), because you also have the Border<side>Foreground() methods, which can conflict. I have already been tripped up a few times in the past with both <prop>() and <side><prop>() existing for some of the methods (and the method documentation helps dispell the confusion).

• Could also have the BorderForeground() and Border<side>Foreground() methods wipe the value of blend, as it's only 1 directional at the moment.

Not opposed to a new type, but trying to accept either (blend type or color.Color) into a method seems like it would increase confusion. I can't think of anything better, but I'll keep trying to brainstorm ideas.

[caarlos0]

this overall looks to me, thanks for the awesome PR!

one comment i have which might be a matter of preference is: I rather not have the wrap option.

e.g.:

		BorderBlendForeground(
			charmtone.Cherry,
			charmtone.Charple,
			charmtone.Guac,
			charmtone.Charple,
			charmtone.Sriracha,
		).

With using BorderBlendWrap, it'll repeat the sriracha color, right?

I think this rarely will have enough colors that it's worth not having control on how to "wrap", e.g. the caller can simply repeat the colors in the inverse order as they see fit (repeating the middle one or not).

[lrstanley]

With using BorderBlendWrap, it'll repeat the sriracha color, right?

I think this rarely will have enough colors that it's worth not having control on how to "wrap", e.g. the caller can simply repeat the colors in the inverse order as they see fit (repeating the middle one or not).

Yes, it will duplicate all of the colors, just in reverse order. Removing wrap does simplify things and reduces added methods, will just have to ensure examples guide folks in the right direction so it's obvious that users should effectively do the same themselves. If that's cool with others, will update soon to remove wrap and further update examples & docs.

[meowgorithm]

That sounds good: we can remove the method now to reduce surface area. I might even just un-expose it for now, since the code is written so we can flip it on later if we feel the need (or if we don't, we can ultimately remove it).

Also, do we need to makeBorderBlendOffset specific to the foreground? If we ever add background blends we may as well leave the door open for them to be set independently. If you both agree that that makes sense, I suggest we name the two methods to BorderForegroundBlend and BorderForegroundBlendOffset in order for LSPs to match against the BorderForeground for API discoverability.

We're almost there, otherwise. Thanks for tolerating my persnickety comments.

[lrstanley]

That sounds good: we can remove the method now to reduce surface area. I might even just un-expose it for now, since the code is written so we can flip it on later if we feel the need (or if we don't, we can ultimately remove it).

I've removed it for now to keep the code clean, since it should be easy to add back in the future if desired. Added some documentation as well to help drive users in the right direction on how to use the set method without the use of wrapping.

Also, do we need to makeBorderBlendOffset specific to the foreground? If we ever add background blends we may as well leave the door open for them to be set independently. If you both agree that that makes sense, I suggest we name the two methods to BorderForegroundBlend and BorderForegroundBlendOffset in order for LSPs to match against the BorderForeground for API discoverability.

I've updated to both of these names, and all example code.

[meowgorithm]

Looks good to me. Let get this in.