Border Styles in Themes

[smbroadley]

Added border styling support to themes. Currently only on ui.popup.info for preview.
Added support for extracting parts of a style in the scope-chains. This is used to resolve the 'border_type' value.

The theme TOML schema should allow for flexibility in the future by switching the meaning of the string to be a 'lines collection' theme lookup (like what happens for colours, and the palette definition).

I realize this adds more storage to the styles, and will only be used in a couple of scopes, but it should be fairly negligible.

Feedback is welcome, as this is my first contribution, and I am still trying to build a mental map of the codebase. 😄

[pascalkuthe]

I am not sure if border styles should be part of the them. A Style is something very fundamental in helix that control how a character/grapheme is rendered. I dont think it makes sense to add the border style to that because that control what character/grapheme is rendered not how.

This also shows in the implementation/changes you had to do. Maybe this could just be a normal config option? This seems somewhat similar to #5371 (ignore that it's closed I want yo revive that PR at some point). I think just allowing the character to be set makes more sense here instead of hardcoding options. It's not that many characters that would be changed.

Something similar recently also came up in #6417 btw. and I am planning to use a similar config there as I described above.

[smbroadley]

I added it to themes as I felt border styling is a stylistic decision. Maybe the theme schema should be extended for new theme options (like this, and gutter symbols, etc...)?

Indeed, I was worried about the longevity of using the 'preset' names, but that is easily mitigated (as I mentioned) by making that look-up a line set, either defined as a built-in, or in the theme TOML file (like palette entries).

Thoughts?

[pascalkuthe]

Indeed, I was worried about the longevity of using the 'preset' names, but that is easily mitigated (as I mentioned) by making that look-up a line set, either defined as a built-in, or in the theme TOML file (like palette entries).

That just sounds way too complicated. With line sets you would then have multiple config options for the same things. We generally want to keep the TOML config as simple as possible as it's just a holdover until a script-based config land.

We currently have a very clear split that themes define how something is rendered but not what is rendered and I don't think we should change that. There are already options like this to change the character to be rendered in the normal config (for example indent guides character is configurable which is also a line and therefore very similar to what is done here, another example is whitespace chars).

[pascalkuthe]

closing this PR as stale, I don't see us going forward with something like this in the future (see above)