charmbracelet / lipgloss

Style definitions for nice terminal layouts 👄
MIT License
7.89k stars 225 forks source link

Inherit seems misleading #70

Closed Evertras closed 2 years ago

Evertras commented 2 years ago

The comment for Inherit states:

// Inherit takes values from the style in the argument applies them to this
// style, overwriting existing definitions. Only values explicitly set on the
// style in argument will be applied.

But the code seems to behave in the opposite way: https://github.com/charmbracelet/lipgloss/blob/master/style.go#L146

The code skips existing definitions, it does not overwrite. Is this intentional?

meowgorithm commented 2 years ago

You're absolutely right, this is poorly worded. The implementation is as-intended though. The idea is that, say, if you had a background color at the top level you could cascade it to children unless one of the children has a background color set.

Congrats on making Golang Weekly this week, btw!

Evertras commented 2 years ago

Got it! I managed to get the desired behavior for exactly that kind of use case (base -> column -> row -> cell precedence of style application), I just had to reverse the order of the calls. I can offer a quick PR to reword the comment for clarity.

And thanks! Been a lot of fun working in this ecosystem, looking forward to more!

meowgorithm commented 2 years ago

Wonderful. By all means, please do submit a PR — !