Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow separate styles for markup headings #1618

Merged
merged 2 commits into from
Feb 21, 2022
Merged

Allow separate styles for markup headings #1618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3ab978a
update markdown highlighting to use separate heading themes
CptPotato Feb 3, 2022
fda1c76
remove markdown theme scopes in ui
CptPotato Feb 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions book/src/themes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,8 @@ We use a similar set of scopes as

- `markup`
- `heading`
- `marker`
- `1`, `2`, `3`, `4`, `5`, `6` - heading text for h1 through h6
- `list`
- `unnumbered`
- `numbered`
Expand Down
3 changes: 1 addition & 2 deletions helix-term/src/commands/lsp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,8 +632,7 @@ pub fn hover(cx: &mut Context) {

// skip if contents empty

let contents =
ui::Markdown::new(contents, editor.syn_loader.clone()).style_group("hover");
let contents = ui::Markdown::new(contents, editor.syn_loader.clone());
let popup = Popup::new("hover", contents);
compositor.replace_or_push("hover", Box::new(popup));
}
Expand Down
8 changes: 3 additions & 5 deletions helix-term/src/ui/completion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,16 +305,14 @@ impl Component for Completion {
let coords = helix_core::visual_coords_at_pos(text, cursor_pos, doc.tab_width());
let cursor_pos = (coords.row - view.offset.row) as u16;

let markdown_ui =
|content, syn_loader| Markdown::new(content, syn_loader).style_group("completion");
let mut markdown_doc = match &option.documentation {
Some(lsp::Documentation::String(contents))
| Some(lsp::Documentation::MarkupContent(lsp::MarkupContent {
kind: lsp::MarkupKind::PlainText,
value: contents,
})) => {
// TODO: convert to wrapped text
markdown_ui(
Markdown::new(
format!(
"```{}\n{}\n```\n{}",
language,
Expand All @@ -329,7 +327,7 @@ impl Component for Completion {
value: contents,
})) => {
// TODO: set language based on doc scope
markdown_ui(
Markdown::new(
format!(
"```{}\n{}\n```\n{}",
language,
Expand All @@ -343,7 +341,7 @@ impl Component for Completion {
// TODO: copied from above

// TODO: set language based on doc scope
markdown_ui(
Markdown::new(
format!(
"```{}\n{}\n```",
language,
Expand Down
62 changes: 36 additions & 26 deletions helix-term/src/ui/markdown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,45 +6,46 @@ use tui::{

use std::sync::Arc;

use pulldown_cmark::{CodeBlockKind, CowStr, Event, Options, Parser, Tag};
use pulldown_cmark::{CodeBlockKind, CowStr, Event, HeadingLevel, Options, Parser, Tag};

use helix_core::{
syntax::{self, HighlightEvent, Syntax},
Rope,
};
use helix_view::{
graphics::{Margin, Rect},
graphics::{Margin, Rect, Style},
Theme,
};

pub struct Markdown {
contents: String,

config_loader: Arc<syntax::Loader>,

block_style: String,
heading_style: String,
}

// TODO: pre-render and self reference via Pin
// better yet, just use Tendril + subtendril for references

impl Markdown {
// theme keys, including fallbacks
const TEXT_STYLE: [&'static str; 2] = ["ui.text", "ui"];
const BLOCK_STYLE: [&'static str; 3] = ["markup.raw.inline", "markup.raw", "markup"];
const HEADING_STYLES: [[&'static str; 3]; 6] = [
["markup.heading.1", "markup.heading", "markup"],
["markup.heading.2", "markup.heading", "markup"],
["markup.heading.3", "markup.heading", "markup"],
["markup.heading.4", "markup.heading", "markup"],
["markup.heading.5", "markup.heading", "markup"],
["markup.heading.6", "markup.heading", "markup"],
];

pub fn new(contents: String, config_loader: Arc<syntax::Loader>) -> Self {
Self {
contents,
config_loader,
block_style: "markup.raw.inline".into(),
heading_style: "markup.heading".into(),
}
}

pub fn style_group(mut self, suffix: &str) -> Self {
self.block_style = format!("markup.raw.inline.{}", suffix);
self.heading_style = format!("markup.heading.{}", suffix);
self
}

fn parse(&self, theme: Option<&Theme>) -> tui::text::Text<'_> {
// // also 2021-03-04T16:33:58.553 helix_lsp::transport [INFO] <- {"contents":{"kind":"markdown","value":"\n```rust\ncore::num\n```\n\n```rust\npub const fn saturating_sub(self, rhs:Self) ->Self\n```\n\n---\n\n```rust\n```"},"range":{"end":{"character":61,"line":101},"start":{"character":47,"line":101}}}
// let text = "\n```rust\ncore::iter::traits::iterator::Iterator\n```\n\n```rust\nfn collect<B: FromIterator<Self::Item>>(self) -> B\nwhere\n Self: Sized,\n```\n\n---\n\nTransforms an iterator into a collection.\n\n`collect()` can take anything iterable, and turn it into a relevant\ncollection. This is one of the more powerful methods in the standard\nlibrary, used in a variety of contexts.\n\nThe most basic pattern in which `collect()` is used is to turn one\ncollection into another. You take a collection, call [`iter`](https://doc.rust-lang.org/nightly/core/iter/traits/iterator/trait.Iterator.html) on it,\ndo a bunch of transformations, and then `collect()` at the end.\n\n`collect()` can also create instances of types that are not typical\ncollections. For example, a [`String`](https://doc.rust-lang.org/nightly/core/iter/std/string/struct.String.html) can be built from [`char`](type@char)s,\nand an iterator of [`Result<T, E>`](https://doc.rust-lang.org/nightly/core/result/enum.Result.html) items can be collected\ninto `Result<Collection<T>, E>`. See the examples below for more.\n\nBecause `collect()` is so general, it can cause problems with type\ninference. As such, `collect()` is one of the few times you'll see\nthe syntax affectionately known as the 'turbofish': `::<>`. This\nhelps the inference algorithm understand specifically which collection\nyou're trying to collect into.\n\n# Examples\n\nBasic usage:\n\n```rust\nlet a = [1, 2, 3];\n\nlet doubled: Vec<i32> = a.iter()\n .map(|&x| x * 2)\n .collect();\n\nassert_eq!(vec![2, 4, 6], doubled);\n```\n\nNote that we needed the `: Vec<i32>` on the left-hand side. This is because\nwe could collect into, for example, a [`VecDeque<T>`](https://doc.rust-lang.org/nightly/core/iter/std/collections/struct.VecDeque.html) instead:\n\n```rust\nuse std::collections::VecDeque;\n\nlet a = [1, 2, 3];\n\nlet doubled: VecDeque<i32> = a.iter().map(|&x| x * 2).collect();\n\nassert_eq!(2, doubled[0]);\nassert_eq!(4, doubled[1]);\nassert_eq!(6, doubled[2]);\n```\n\nUsing the 'turbofish' instead of annotating `doubled`:\n\n```rust\nlet a = [1, 2, 3];\n\nlet doubled = a.iter().map(|x| x * 2).collect::<Vec<i32>>();\n\nassert_eq!(vec![2, 4, 6], doubled);\n```\n\nBecause `collect()` only cares about what you're collecting into, you can\nstill use a partial type hint, `_`, with the turbofish:\n\n```rust\nlet a = [1, 2, 3];\n\nlet doubled = a.iter().map(|x| x * 2).collect::<Vec<_>>();\n\nassert_eq!(vec![2, 4, 6], doubled);\n```\n\nUsing `collect()` to make a [`String`](https://doc.rust-lang.org/nightly/core/iter/std/string/struct.String.html):\n\n```rust\nlet chars = ['g', 'd', 'k', 'k', 'n'];\n\nlet hello: String = chars.iter()\n .map(|&x| x as u8)\n .map(|x| (x + 1) as char)\n .collect();\n\nassert_eq!(\"hello\", hello);\n```\n\nIf you have a list of [`Result<T, E>`](https://doc.rust-lang.org/nightly/core/result/enum.Result.html)s, you can use `collect()` to\nsee if any of them failed:\n\n```rust\nlet results = [Ok(1), Err(\"nope\"), Ok(3), Err(\"bad\")];\n\nlet result: Result<Vec<_>, &str> = results.iter().cloned().collect();\n\n// gives us the first error\nassert_eq!(Err(\"nope\"), result);\n\nlet results = [Ok(1), Ok(3)];\n\nlet result: Result<Vec<_>, &str> = results.iter().cloned().collect();\n\n// gives us the list of answers\nassert_eq!(Ok(vec![1, 3]), result);\n```";
Expand All @@ -67,17 +68,19 @@ impl Markdown {
})
}

macro_rules! get_theme {
($s1: expr) => {
theme
.map(|theme| theme.try_get($s1.as_str()))
.flatten()
.unwrap_or_default()
};
}
let text_style = theme.map(|theme| theme.get("ui.text")).unwrap_or_default();
let code_style = get_theme!(self.block_style);
let heading_style = get_theme!(self.heading_style);
let get_theme = |keys: &[&str]| match theme {
Some(theme) => keys
.iter()
.find_map(|key| theme.try_get(key))
.unwrap_or_default(),
None => Default::default(),
};
let text_style = get_theme(&Self::TEXT_STYLE);
let code_style = get_theme(&Self::BLOCK_STYLE);
let heading_styles: Vec<Style> = Self::HEADING_STYLES
.iter()
.map(|key| get_theme(key))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@CptPotato CptPotato Feb 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

.collect();

for event in parser {
match event {
Expand Down Expand Up @@ -172,9 +175,16 @@ impl Markdown {
lines.push(Spans::from(span));
}
}
} else if let Some(Tag::Heading(_, _, _)) = tags.last() {
} else if let Some(Tag::Heading(level, _, _)) = tags.last() {
let mut span = to_span(text);
span.style = heading_style;
span.style = match level {
HeadingLevel::H1 => heading_styles[0],
HeadingLevel::H2 => heading_styles[1],
HeadingLevel::H3 => heading_styles[2],
HeadingLevel::H4 => heading_styles[3],
HeadingLevel::H5 => heading_styles[4],
HeadingLevel::H6 => heading_styles[5],
};
spans.push(span);
} else {
let mut span = to_span(text);
Expand Down
13 changes: 9 additions & 4 deletions runtime/queries/markdown/highlights.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
[
(atx_heading)
(setext_heading)
] @markup.heading
(setext_heading (heading_content) @markup.heading.1 (setext_h1_underline) @markup.heading.marker)
(setext_heading (heading_content) @markup.heading.2 (setext_h2_underline) @markup.heading.marker)

(atx_heading (atx_h1_marker) @markup.heading.marker (heading_content) @markup.heading.1)
(atx_heading (atx_h2_marker) @markup.heading.marker (heading_content) @markup.heading.2)
(atx_heading (atx_h3_marker) @markup.heading.marker (heading_content) @markup.heading.3)
(atx_heading (atx_h4_marker) @markup.heading.marker (heading_content) @markup.heading.4)
(atx_heading (atx_h5_marker) @markup.heading.marker (heading_content) @markup.heading.5)
(atx_heading (atx_h6_marker) @markup.heading.marker (heading_content) @markup.heading.6)

(code_fence_content) @none

Expand Down