Tree-sitter tokens don't have any background colour

[Romankivs]

Describe the bug

Tree-sitter tokens don't have any background colour.

To Reproduce

Steps to reproduce the behavior:

1. Setup tree-sitter grammar and parser for any language (e.g Rust) in ~/.ad.
2. Open any file with code written in the corresponding language.

Expected behavior

Tree-sitter tokens are display with default background colour.

Screenshots

How it looks:

Versions & OS Details

• OS: Linux
• Distribution: Arch
• ad Version: 7971f28

[Romankivs]

Proposed fix: #82

[sminez]

Rather than requiring a background colour to be specified for each syntax element, I'd prefer that the default background be used when one is missing. Otherwise all this does is "fix" the default config.

For future issues, please bear in mind the advice given in the README of this repo and wait for a response to issues before raising PRs.

[Romankivs]

For future issues, please bear in mind the advice given in the README of this repo and wait for a response to issues before raising PRs.

Sure, no problem

I'd prefer that the default background be used when one is missing.

Yeah, seems reasonable.

[Romankivs]

The most straightforward way to achieve this seems to be to modify Colorscheme::styles_for method (responsible for providing Styles for a given tree-sitter token type) to insert default colorscheme's bg color when none is specified for a given token type:

pub fn styles_for(&self, tag: &str) -> Styles {
    let mut styles = match self.syntax.get(tag) {
        Some(styles) => styles,
        None => tag
            .split_once('.')
            .and_then(|(prefix, _)| self.syntax.get(prefix))
            .or(self.syntax.get(TK_DEFAULT))
            .expect("to have default styles"),
    }
    .clone();

    // Use default colorscheme's background color if none is specified
    if styles.bg.is_none() {
        styles.bg = Some(self.bg);
    }

    styles
}

Let me know whether this is an acceptable solution for this issue.

[sminez]

I think that looks mostly alright but can we make the check and set the default once after loading config rather than on each access? This code gets called pretty frequently so keeping things as efficient as possible is a priority. There's definitely outstanding work around making this more efficient in general but I'd at least like to maintain the current performance where possible.

[Romankivs]

The right place for setting default bg color for tree-sitter tokens seems to be inside of Config::try_load() near cfg.expand_home_dir_refs(&home); that is also responsible for config modification before returning it.

pub fn try_load() -> Result<Self, String> {
  ...
  
  cfg.set_default_bg_for_tokens();
  cfg.expand_home_dir_refs(&home);
  
  Ok(cfg)
}

...

fn set_default_bg_for_tokens(&mut self) {
    for style in self.colorscheme.syntax.values_mut() {
        // Use default colorscheme's background color if none is specified
        if style.bg.is_none() {
            style.bg = Some(self.colorscheme.bg);
        }
    }
}

Let me know whether my assumptions are correct and that is similar to what you had in mind.

[sminez]

That looks reasonable 👍

[Romankivs]

PR: #86

[sminez]

Sorry @Romankivs, I didn't see that PR come through in my GitHub notifications. Thank you for this!