From d25c6d80c626adacd05a7238f0aaf42c160a58b4 Mon Sep 17 00:00:00 2001 From: Nick Groenen Date: Sun, 9 Jan 2022 12:52:42 +0100 Subject: [PATCH] Chg: Pass context and events as mutable references to postprocessors Instead of passing clones of context and the markdown tree to postprocessors, pass them a mutable reference which may be modified in-place. This is a breaking change to the postprocessor implementation, changing both the input arguments as well as the return value: ```diff - dyn Fn(Context, MarkdownEvents) -> (Context, MarkdownEvents, PostprocessorResult) + Send + Sync; + dyn Fn(&mut Context, &mut MarkdownEvents) -> PostprocessorResult + Send + Sync; ``` With this change the postprocessor API becomes a little more ergonomic to use however, especially making the intent around return statements more clear. --- src/lib.rs | 41 ++++++++++------------------ src/postprocessors.rs | 20 +++++++------- tests/postprocessors_test.rs | 52 ++++++++++++++---------------------- 3 files changed, 43 insertions(+), 70 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index fcf7ae0..cdb44f2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -74,9 +74,8 @@ pub type MarkdownEvents<'a> = Vec>; /// defined inline as a closure. /// /// ``` -/// use obsidian_export::{Context, Exporter, MarkdownEvents, PostprocessorResult}; -/// use obsidian_export::pulldown_cmark::{CowStr, Event}; /// use obsidian_export::serde_yaml::Value; +/// use obsidian_export::{Exporter, PostprocessorResult}; /// # use std::path::PathBuf; /// # use tempfile::TempDir; /// @@ -86,7 +85,7 @@ pub type MarkdownEvents<'a> = Vec>; /// let mut exporter = Exporter::new(source, destination); /// /// // add_postprocessor registers a new postprocessor. In this example we use a closure. -/// exporter.add_postprocessor(&|mut context, events| { +/// exporter.add_postprocessor(&|context, _events| { /// // This is the key we'll insert into the frontmatter. In this case, the string "foo". /// let key = Value::String("foo".to_string()); /// // This is the value we'll insert into the frontmatter. In this case, the string "bar". @@ -95,9 +94,8 @@ pub type MarkdownEvents<'a> = Vec>; /// // Frontmatter can be updated in-place, so we can call insert on it directly. /// context.frontmatter.insert(key, value); /// -/// // Postprocessors must return their (modified) context, the markdown events that make -/// // up the note and a next action to take. -/// (context, events, PostprocessorResult::Continue) +/// // This return value indicates processing should continue. +/// PostprocessorResult::Continue /// }); /// /// exporter.run().unwrap(); @@ -117,18 +115,13 @@ pub type MarkdownEvents<'a> = Vec>; /// # use tempfile::TempDir; /// # /// /// This postprocessor replaces any instance of "foo" with "bar" in the note body. -/// fn foo_to_bar( -/// context: Context, -/// events: MarkdownEvents, -/// ) -> (Context, MarkdownEvents, PostprocessorResult) { -/// let events = events -/// .into_iter() -/// .map(|event| match event { -/// Event::Text(text) => Event::Text(CowStr::from(text.replace("foo", "bar"))), -/// event => event, -/// }) -/// .collect(); -/// (context, events, PostprocessorResult::Continue) +/// fn foo_to_bar(context: &mut Context, events: &mut MarkdownEvents) -> PostprocessorResult { +/// for event in events.iter_mut() { +/// if let Event::Text(text) = event { +/// *event = Event::Text(CowStr::from(text.replace("foo", "bar"))) +/// } +/// } +/// PostprocessorResult::Continue /// } /// /// # let tmp_dir = TempDir::new().expect("failed to make tempdir"); @@ -140,7 +133,7 @@ pub type MarkdownEvents<'a> = Vec>; /// ``` pub type Postprocessor = - dyn Fn(Context, MarkdownEvents) -> (Context, MarkdownEvents, PostprocessorResult) + Send + Sync; + dyn Fn(&mut Context, &mut MarkdownEvents) -> PostprocessorResult + Send + Sync; type Result = std::result::Result; const PERCENTENCODE_CHARS: &AsciiSet = &CONTROLS.add(b' ').add(b'(').add(b')').add(b'%').add(b'?'); @@ -407,10 +400,7 @@ impl<'a> Exporter<'a> { let (frontmatter, mut markdown_events) = self.parse_obsidian_note(src, &context)?; context.frontmatter = frontmatter; for func in &self.postprocessors { - let res = func(context, markdown_events); - context = res.0; - markdown_events = res.1; - match res.2 { + match func(&mut context, &mut markdown_events) { PostprocessorResult::StopHere => break, PostprocessorResult::StopAndSkipNote => return Ok(()), PostprocessorResult::Continue => (), @@ -615,10 +605,7 @@ impl<'a> Exporter<'a> { for func in &self.embed_postprocessors { // Postprocessors running on embeds shouldn't be able to change frontmatter (or // any other metadata), so we give them a clone of the context. - let res = func(child_context, events); - child_context = res.0; - events = res.1; - match res.2 { + match func(&mut child_context, &mut events) { PostprocessorResult::StopHere => break, PostprocessorResult::StopAndSkipNote => { events = vec![]; diff --git a/src/postprocessors.rs b/src/postprocessors.rs index 8c3fb3a..0e8141e 100644 --- a/src/postprocessors.rs +++ b/src/postprocessors.rs @@ -6,15 +6,13 @@ use pulldown_cmark::Event; /// This postprocessor converts all soft line breaks to hard line breaks. Enabling this mimics /// Obsidian's _'Strict line breaks'_ setting. pub fn softbreaks_to_hardbreaks( - context: Context, - events: MarkdownEvents, -) -> (Context, MarkdownEvents, PostprocessorResult) { - let events = events - .into_iter() - .map(|event| match event { - Event::SoftBreak => Event::HardBreak, - _ => event, - }) - .collect(); - (context, events, PostprocessorResult::Continue) + _context: &mut Context, + events: &mut MarkdownEvents, +) -> PostprocessorResult { + for event in events.iter_mut() { + if event == &Event::SoftBreak { + *event = Event::HardBreak; + } + } + PostprocessorResult::Continue } diff --git a/tests/postprocessors_test.rs b/tests/postprocessors_test.rs index bc95318..ef701b6 100644 --- a/tests/postprocessors_test.rs +++ b/tests/postprocessors_test.rs @@ -8,30 +8,22 @@ use std::path::PathBuf; use tempfile::TempDir; /// This postprocessor replaces any instance of "foo" with "bar" in the note body. -fn foo_to_bar( - ctx: Context, - events: MarkdownEvents, -) -> (Context, MarkdownEvents, PostprocessorResult) { - let events = events - .into_iter() - .map(|event| match event { - Event::Text(text) => Event::Text(CowStr::from(text.replace("foo", "bar"))), - event => event, - }) - .collect(); - (ctx, events, PostprocessorResult::Continue) +fn foo_to_bar(_ctx: &mut Context, events: &mut MarkdownEvents) -> PostprocessorResult { + for event in events.iter_mut() { + if let Event::Text(text) = event { + *event = Event::Text(CowStr::from(text.replace("foo", "bar"))) + } + } + PostprocessorResult::Continue } /// This postprocessor appends "bar: baz" to frontmatter. -fn append_frontmatter( - mut ctx: Context, - events: MarkdownEvents, -) -> (Context, MarkdownEvents, PostprocessorResult) { +fn append_frontmatter(ctx: &mut Context, _events: &mut MarkdownEvents) -> PostprocessorResult { ctx.frontmatter.insert( Value::String("bar".to_string()), Value::String("baz".to_string()), ); - (ctx, events, PostprocessorResult::Continue) + PostprocessorResult::Continue } // The purpose of this test to verify the `append_frontmatter` postprocessor is called to extend @@ -62,9 +54,8 @@ fn test_postprocessor_stophere() { tmp_dir.path().to_path_buf(), ); - exporter.add_postprocessor(&|ctx, mdevents| (ctx, mdevents, PostprocessorResult::StopHere)); - exporter - .add_embed_postprocessor(&|ctx, mdevents| (ctx, mdevents, PostprocessorResult::StopHere)); + exporter.add_postprocessor(&|_ctx, _mdevents| PostprocessorResult::StopHere); + exporter.add_embed_postprocessor(&|_ctx, _mdevents| PostprocessorResult::StopHere); exporter.add_postprocessor(&|_, _| panic!("should not be called due to above processor")); exporter.add_embed_postprocessor(&|_, _| panic!("should not be called due to above processor")); exporter.run().unwrap(); @@ -84,8 +75,7 @@ fn test_postprocessor_stop_and_skip() { assert!(note_path.exists()); remove_file(¬e_path).unwrap(); - exporter - .add_postprocessor(&|ctx, mdevents| (ctx, mdevents, PostprocessorResult::StopAndSkipNote)); + exporter.add_postprocessor(&|_ctx, _mdevents| PostprocessorResult::StopAndSkipNote); exporter.run().unwrap(); assert!(!note_path.exists()); @@ -104,9 +94,9 @@ fn test_postprocessor_change_destination() { assert!(original_note_path.exists()); remove_file(&original_note_path).unwrap(); - exporter.add_postprocessor(&|mut ctx, mdevents| { + exporter.add_postprocessor(&|ctx, _mdevents| { ctx.destination.set_file_name("MovedNote.md"); - (ctx, mdevents, PostprocessorResult::Continue) + PostprocessorResult::Continue }); exporter.run().unwrap(); @@ -147,9 +137,7 @@ fn test_embed_postprocessors_stop_and_skip() { PathBuf::from("tests/testdata/input/postprocessors"), tmp_dir.path().to_path_buf(), ); - exporter.add_embed_postprocessor(&|ctx, mdevents| { - (ctx, mdevents, PostprocessorResult::StopAndSkipNote) - }); + exporter.add_embed_postprocessor(&|_ctx, _mdevents| PostprocessorResult::StopAndSkipNote); exporter.run().unwrap(); @@ -171,9 +159,9 @@ fn test_embed_postprocessors_context() { tmp_dir.path().to_path_buf(), ); - exporter.add_postprocessor(&|ctx, mdevents| { + exporter.add_postprocessor(&|ctx, _mdevents| { if ctx.current_file() != &PathBuf::from("Note.md") { - return (ctx, mdevents, PostprocessorResult::Continue); + return PostprocessorResult::Continue; } let is_root_note = ctx .frontmatter @@ -188,9 +176,9 @@ fn test_embed_postprocessors_context() { &ctx.current_file().display() ) } - (ctx, mdevents, PostprocessorResult::Continue) + PostprocessorResult::Continue }); - exporter.add_embed_postprocessor(&|ctx, mdevents| { + exporter.add_embed_postprocessor(&|ctx, _mdevents| { let is_root_note = ctx .frontmatter .get(&Value::String("is_root_note".to_string())) @@ -204,7 +192,7 @@ fn test_embed_postprocessors_context() { &ctx.current_file().display() ) } - (ctx, mdevents, PostprocessorResult::Continue) + PostprocessorResult::Continue }); exporter.run().unwrap();