feat(config): allow configuring widget border styles

[raylu]

📺 PR Description

this allows changing (or removing) the border styles

tv 0.12.3:

this PR with

[ui.input_bar]
border_type = 'none'
[ui.results_panel]
border_type = 'none'
[ui.preview_panel]
border_type = 'plain'

see #638

Checklist

• my commits and PR title follow the conventional commits format
• if this is a new feature, I have added tests to consolidate the feature and prevent regressions
• if this is a bug fix, I have added a test that reproduces the bug (if applicable)
• I have added a reasonable amount of documentation to the code where appropriate

[alexpasmantier]

Awesome, good job with getting that up and running so quickly.

I think border configuration should go in television/config/ui instead of television/config/themes which should really only deal with colors.

e.g.

#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Hash)]
#[serde(default)]
pub struct PreviewPanelConfig {
    pub size: u16,
    pub header: Option<Template>,
    pub footer: Option<Template>,
    pub scrollbar: bool,
    pub border_style: BorderStyle,
}

The only thing with that being that we currently don't have equivalent structs for ResultsPanelConfig etc. but I can help refactor that if needed.

Also, you can use serde to avoid having to manually implement deserialization for your BorderType:

#[derive(Clone, Debug, PartialEq, Deserialize, Default)]
#[serde(rename_all = "snake_case")]
pub enum BorderType {
    None,
    Plain,
    #[default] // Default to Rounded
    Rounded,
    Thick,
}

[raylu]

tests are failing left and right, but let me know if this implementation makes sense

[alexpasmantier]

tests are failing left and right, but let me know if this implementation makes sense

This looks much better 👍🏻

[alexpasmantier]

#648 is also on the way and making changes to similar files. I plan to merge that one relatively quickly and I thought we might as well refactor that as well in your PR if you don't mind 😊

[raylu]

sure, if it lands first I'll deal with the conflicts and bring in input_prompt

[raylu]

ok, I've rebased atop e809ccf

[alexpasmantier]

Nice 👌🏻

The config part works perfectly.

I'm unable however to set anything through channel prototypes.

Both:

[ui.results_panel]
border_type = "none"

and

[ui]
results_panel = { border_type = "none" }

seem to have no effect. Same for the other borders.

This makes me think we might want to add more tests including integration tests where we check that a given config and a given prototype render to the expected UI borders. TV has its own micro-framework for these kinds of tests and you can draw inspiration from one of the many existing ones (e.g. https://github.com/raylu/television/blob/2e388ce8151573dc4d79da84ff89220685c2ae4e/tests/cli/cli_ui.rs#L10-L31).

[alexpasmantier]

Oh and it goes without saying the code looks much nicer and cleaner with that refactor you did. Thanks for that 🙏🏻

[raylu]

that's... very odd. here's me running cargo r with

[ui.input_bar]
border_type = 'none'
[ui.results_panel]
border_type = 'none'
[ui.preview_panel]
border_type = 'plain'

and without, side-by-side

[alexpasmantier]

I'm using a config.toml without any border settings.

I have the following text channel:

[metadata]
name = "text"
description = "A channel to find and select text from files"
requirements = ["rg", "bat"]

[source]
command = "rg . --no-heading --line-number --colors 'match:fg:white' --colors 'path:fg:blue' --color=always"
ansi = true
output = "{strip_ansi|split:\\::..2}"

[preview]
command = "bat -n --color=always '{strip_ansi|split:\\::0}'"
env = { BAT_THEME = "ansi" }
offset = '{strip_ansi|split:\::1}'

[ui.preview_panel]
border_type = 'none'
header = '{strip_ansi|split:\::..2}'

Then run:

gh pr checkout 642
cargo run -- --text

And get the following that has borders around the preview:

Am I missing something? 🤔

[lalvarezt]

looks good, if you don't mind me intruding. Have you considered the possibility of adding also padding as part of the config? Right now if you remove the borders the text takes over that place and it might be too close to the edge. Adding optional padding would help to overcome this, something like

 Add padding configuration alongside the existing border_type fields:

  // In config/ui.rs
  pub struct InputBarConfig {
      pub border_type: BorderType,
      pub padding: PaddingConfig,  // <- New field
  }

  pub struct ResultsPanelConfig {
      pub border_type: BorderType,
      pub padding: PaddingConfig,  // <- New field
  }

  pub struct PreviewPanelConfig {
      pub border_type: BorderType,
      pub padding: PaddingConfig,  // <- New field
  }

  #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Hash, Default)]
  pub struct PaddingConfig {
      pub top: u16,
      pub right: u16,
      pub bottom: u16,
      pub left: u16,
  }

This would allow config like:

  [ui.input_bar]
  border_type = "none"
  padding = { top = 1, right = 1, bottom = 1, left = 1 }

wdty?

[raylu]

probably not in this PR as it's already doing quite a bit

also just noticed I have conflicts again

[alexpasmantier]

Ended up adding the CLI options for BorderTypes and config + prototypes + cli for Padding as well.

[alexpasmantier]

And fixed the conflicts.

I'm happy to merge this as is if you're okay with it @lalvarezt @raylu :-)

I do feel like we need to address the configuration logic problem which is quite a mess right now and is starting to get difficult to maintain, especially overrides (cli, config, prototypes).

Maybe @raylu's suggestion https://docs.rs/merge/latest/merge/ can help with that.

[lalvarezt]

I do feel like we need to address the configuration logic problem which is quite a mess right now and is starting to get difficult to maintain, especially overrides (cli, config, prototypes).

Maybe @raylu's suggestion https://docs.rs/merge/latest/merge/ can help with that.

yup, right with you there, it's getting complex with so many options and priorities

[raylu]

oh interesting. I didn't think anyone would want to mess with borders through argv, but I guess I could see it

yeah, feel free to merge!