Allow separate styles for markup headings

[CptPotato]

This makes it possible to use separate styles for each markup heading level:

I've never worked with tree sitter before, so someone might want to confirm that highlights.scm is okay.

adresses #1597

[CptPotato]

I ended up updating helix-term/src/ui/markdown.rs aswell, thinking it was required for markdown highlighting to work but it looks like it isn't 👀

I'm not 100% sure what it is used for. (edit: it's used for doc and completion popups)

[sudormrfbin]

I ended up updating helix-term/src/ui/markdown.rs aswell, thinking it was required for markdown highlighting to work but it looks like it isn't eyes

I'm not 100% sure what it is used for.

It's used for rendering markdown in the completion and documentation popups btw.

[CptPotato]

It's used for rendering markdown in the completion and documentation popups btw.

Thanks, The popups seemed to work as expected

Edit: actually, the line that overrides self.heading_styles in style_group() breaks the highlighting in the popup.
If I remove that part the colors are correct again. I'm a bit confused, I hope someone can explain the behavior of style_group().

---

I think I get it now: There's markup.[...].completion and markup.[...].hover.

hover actually refers to the doc popup, so I need to set markup.heading.1.hover in the theme to make it work.
Still, it's odd that it doesn't fall back to markup.heading.1 when it's missing.

[sudormrfbin]

Still, it's odd that it doesn't fall back to markup.heading.1 when it's missing.

It works for syntax highlight scopes automatically but IIRC it has to be done manually for ui theming as of now (here we are manually applying the styles in the popup markdown component even though it's technically syntax highlighting).

[sudormrfbin]

Tested and works.

[archseer]

This has no fallback though, if markup.heading.1 isn't defined, just markup.heading it won't highlight.

[archseer]

The macro would have to be modified to try get_theme(key, "markup.heading") in order. We can also simplify and just use markup.heading for this component.

[CptPotato]

Not sure if recursion is a good idea but something along those lines should work:

// example program

use std::collections::HashMap;

fn get_theme<'a, Y>(theme: &'a HashMap<&str, Y>, key: &str) -> Option<&'a Y> {
    theme
        .get(key)
        .or_else(|| key.rfind(".").and_then(|n| get_theme(theme, &key[..n])))
}

fn main() {
    let mut theme = HashMap::new();

    theme.insert("markup.heading", 1);
    theme.insert("markup.heading.2", 2);

    assert_eq!(get_theme(&theme, "markup.heading.1"), Some(&1));
    assert_eq!(get_theme(&theme, "markup.heading.2"), Some(&2));
    assert_eq!(get_theme(&theme, "markup.bold"), None);
}

Edit: Note that this performs a fallback rather than an override. Though, overriding shouldn't be much more difficult either.

Alternatively, the fallbacks for every key (Vec<String>) could be specified manually in markdown.rs or generated in the constructor, if this is a rather hot function.

I'm not very familiar with macros, sorry. Do they have a performance/convinience advantage in this case?

[archseer]

Yeah I don't think there was any reason for macros, personally in not sure if we really need the namespacing for different popups #1363

Here's a similar method in syntax.rs, perhaps we can use the same algorithm:

helix/helix-core/src/syntax.rs

Lines 1197 to 1211 in d3221b0

	for (i, recognized_name) in recognized_names.iter().enumerate() {
	let recognized_name = recognized_name;
	let mut len = 0;
	let mut matches = true;
	for part in recognized_name.split('.') {
	len += 1;
	if !capture_parts.contains(&part) {
	matches = false;
	break;
	}
	}
	if matches && len > best_match_len {
	best_index = Some(i);
	best_match_len = len;
	}

[sudormrfbin]

Yeah I don't think there was any reason for macros, personally in not sure if we really need the namespacing for different popups.

I would be in favour of removing the namespacing part, it seems unnecessary and too granular to be widely used in themes.

[CptPotato]

I would be in favour of removing the namespacing part, it seems unnecessary and too granular to be widely used in themes.

Sorry for the delay. I went ahead and removed the namespacing of the markdown theme keys in the ui. Things seem to work but let me know if I missed anything.

Now that the keys and fallbacks don't change during runtime, I ended up turning them into constants. It's not the prettiest but I didn't have much luck with using macros to make it less manual.

---

Here's a similar method in syntax.rs, perhaps we can use the same algorithm:

helix/helix-core/src/syntax.rs

Lines 1197 to 1211 in d3221b0

	for (i, recognized_name) in recognized_names.iter().enumerate() {
	let recognized_name = recognized_name;
	let mut len = 0;
	let mut matches = true;
	for part in recognized_name.split('.') {
	len += 1;
	if !capture_parts.contains(&part) {
	matches = false;
	break;
	}
	}
	if matches && len > best_match_len {
	best_index = Some(i);
	best_match_len = len;
	}

I might take a look at that at some point. Right now I don't know of a quick way to see if my changes there produce the correct results.

[archseer]

Note: This doesn't fully address all the things in #1597

[archseer]

Tested locally 👍🏻