lifetimes

[Redhawk18]

Hi, the last big problem I was having is with the lifetimes with syntax. I wanted to ask why do we need a reference? Why can't the highlighter type take ownership of the variables required?

pub struct Loader {}

impl Loader {
    pub fn new() -> Self {
        Self {}
    }
}

impl tree_house::LanguageLoader for Loader {
    fn language_for_marker(
        &self,
        marker: tree_house::InjectionLanguageMarker,
    ) -> Option<tree_house::Language> {...}

    fn get_config(&self, lang: tree_house::Language) -> Option<&tree_house::LanguageConfig> {...}
}

pub struct Highlighter(highlighter::Highlighter<'static, 'static, Loader>);

#[derive(Clone, PartialEq)]
pub struct Settings {}

impl text::Highlighter for Highlighter { // <--- static trait bound
    type Settings = Settings;
    type Highlight = highlighter::Highlight;
    type Iterator<'a> = Box<dyn Iterator<Item = (Range<usize>, highlighter::Highlight)> + 'a>; // <--- This would have to turn into b

    fn new(settings: &Self::Settings) -> Self {
        let language = Language(0 as u32);
        let timeout = Duration::from_millis(250);
        let loader = Loader::new();
        let syntax = Syntax::new("todo".into(), language, timeout, &loader)
            .expect("Failed to create syntax");

        Self(highlighter::Highlighter::new(
            &syntax, // <--- 'tree required
            "".into(),
            &loader, // <--- 'a required
            0..5,
        ))
    }

As you can see this quickly becomes a nightmare very fast.

[the-mikedavis]

With tree-house the Syntax type should be long-lived - it should be updated as a document changes - while the highlighter should be very short-lived: only used during rendering. The text::Highlighter trait here looks a little restrictive in what it accepts IMO. To create the highlighter you should pass in the loader and Syntax. Maybe the Settings type can pass that information in.

[Redhawk18]

Why does the highlighter not own its own data? This seems very odd to create a struct with a reference, that's very non-rust and anti-ownership.

[the-mikedavis]

In order to own its data Highlighter would need to consume Syntax, so you could not re-parse and re-highlight if a document changes incrementally (i.e. Syntax::update should generally be much cheaper than Syntax::new). If it owned the Syntax data you could also not run multiple queries at once. Since Highlighter borrows Syntax data you can create multiple Highlighters for the same document and/or other QueryIters that can live and run simultaneously.

I don't agree that this is non-idiomatic. I don't have a set of examples but in my experience this is very common in Rust.

[Redhawk18]

Thats interesting, it seems update takes a slice of edits, so why couldn't it run multiple queries at a time? It seems thats what helix does as well. But thats good to know.

[Redhawk18]

What is the benefit of holding onto the syntax tree your self instead of letting the library handle it?

[the-mikedavis]

I'm not sure what it would look like to have the library handle it. Something needs to hold onto the tree information and storing all of it in a static globally seems less flexible to me.