Skip to content

Postprocessor type does not enable storing state in the postprocessor #175

@rsesek

Description

@rsesek

The type definition for Postprocessor is:

pub type Postprocessor =
    dyn Fn(&mut Context, &mut MarkdownEvents) -> PostprocessorResult + Send + Sync;

When using obsidian-export as a library, this makes it impossible to create a postprocessor that accumulates state as notes are processed. For example, let's say you wanted to collect all the directory parents containing markdown files to produce an index page. The following code:

    let parents: Mutex<HashSet<PathBuf>> = Default::default();
    let callback = |ctx: &mut Context, _events: &mut MarkdownEvents| -> PostprocessorResult {
        let mut dirs = parents.lock().unwrap();
        dirs.insert(ctx.destination.parent().unwrap().to_path_buf());
        PostprocessorResult::Continue
    };
    exporter.add_postprocessor(&callback);

… results in:

error[E0597]: `parents` does not live long enough
  --> src/main.rs:14:28
   |
13 |         |ctx: &mut Context, _events: &mut MarkdownEvents| -> PostprocessorResult {
   |         ------------------------------------------------------------------------ value captured here
14 |             let mut dirs = parents.lock().unwrap();
   |                            ^^^^^^^ borrowed value does not live long enough
...
18 |     exporter.add_postprocessor(&callback);
   |                                --------- cast requires that `parents` is borrowed for `'static`
19 | }
   | - `parents` dropped here while still borrowed

This can be fixed by adding a lifetime annotation to Postprocessor so that 'static is not assumed by the compiler.

But the callback can be somewhat clunky, if the client code is storing state on a struct (as opposed to the example above just capturing a local). E.g., given something like this:

struct State {
    parents: Mutex<HashSet<PathBuf>>,
}

impl State {
    fn new() -> State {
        State {
            parents: Default::default(),
        }
    }
    fn postprocess(&self, ctx: &mut Context, _events: &mut MarkdownEvents) -> PostprocessorResult {
        let mut dirs = self.parents.lock().unwrap();
        dirs.insert(ctx.destination.parent().unwrap().to_path_buf());
        PostprocessorResult::Continue
    }
}

fn main() {
    let mut exporter = Exporter::new(PathBuf::new(), PathBuf::new());
    let state = State::new();
    exporter.add_postprocessor(&|ctx, events| state.postprocess(ctx, events));
    exporter.run().unwrap();
}

The caller needs to move the closure binding to a local given this error:

error[E0716]: temporary value dropped while borrowed
  --> src/main.rs:28:33
   |
28 |     exporter.add_postprocessor(&|ctx, events| state.postprocess(ctx, events));
   |                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - temporary value is freed at the end of this statement
   |                                 |
   |                                 creates a temporary value which is freed while still in use
29 |
30 |     exporter.run().unwrap();
   |     -------------- borrow later used here
   |
help: consider using a `let` binding to create a longer lived value
   |
28 ~     let binding = |ctx, events| state.postprocess(ctx, events);
29 ~     exporter.add_postprocessor(&binding);

It seems like it would be more ergonomic still to provide a trait for postprocessing. I propose addressing both of these: add a lifetime to the Postprocessor type, and define traits for postprocessing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions