New: Report file tree when RecursionLimitExceeded is hit

This refactors the Context to maintain a list of all the files which
have been processed so far in a chain of embeds. This information is
then used to print a more helpful error message to users of the CLI when
RecursionLimitExceeded is returned.
This commit is contained in:
Nick Groenen 2020-12-21 13:01:08 +01:00
parent 7027290697
commit 3b46d6b7d1
No known key found for this signature in database
GPG Key ID: 4F0AD019928AE098
3 changed files with 71 additions and 27 deletions

View File

@ -8,7 +8,7 @@ pub use walker::{vault_contents, WalkOptions};
use pathdiff::diff_paths; use pathdiff::diff_paths;
use percent_encoding::{utf8_percent_encode, AsciiSet, CONTROLS}; use percent_encoding::{utf8_percent_encode, AsciiSet, CONTROLS};
use pulldown_cmark::{CodeBlockKind, CowStr, Event, Options, Parser, Tag}; use pulldown_cmark::{CodeBlockKind, CowStr, Event, Options, Parser, Tag};
use pulldown_cmark_to_cmark::{self, cmark_with_options}; use pulldown_cmark_to_cmark::cmark_with_options;
use rayon::prelude::*; use rayon::prelude::*;
use regex::Regex; use regex::Regex;
use snafu::{ResultExt, Snafu}; use snafu::{ResultExt, Snafu};
@ -27,7 +27,7 @@ lazy_static! {
Regex::new(r"^(?P<file>[^#|]+)(#(?P<block>.+?))??(\|(?P<label>.+?))??$").unwrap(); Regex::new(r"^(?P<file>[^#|]+)(#(?P<block>.+?))??(\|(?P<label>.+?))??$").unwrap();
} }
const PERCENTENCODE_CHARS: &AsciiSet = &CONTROLS.add(b' ').add(b'(').add(b')').add(b'%'); const PERCENTENCODE_CHARS: &AsciiSet = &CONTROLS.add(b' ').add(b'(').add(b')').add(b'%');
const NOTE_RECURSION_LIMIT: u32 = 10; const NOTE_RECURSION_LIMIT: usize = 10;
#[non_exhaustive] #[non_exhaustive]
#[derive(Debug, Snafu)] #[derive(Debug, Snafu)]
@ -57,7 +57,7 @@ pub enum ExportError {
CharacterEncodingError { source: str::Utf8Error }, CharacterEncodingError { source: str::Utf8Error },
#[snafu(display("Recursion limit exceeded"))] #[snafu(display("Recursion limit exceeded"))]
RecursionLimitExceeded {}, RecursionLimitExceeded { file_tree: Vec<PathBuf> },
#[snafu(display("Failed to export '{}'", path.display()))] #[snafu(display("Failed to export '{}'", path.display()))]
FileExportError { FileExportError {
@ -90,35 +90,52 @@ pub struct Exporter<'a> {
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
/// Context holds parser metadata for the file/note currently being parsed. /// Context holds parser metadata for the file/note currently being parsed.
struct Context<'a> { struct Context<'a> {
file: PathBuf, file_tree: Vec<PathBuf>,
vault_contents: &'a [PathBuf], vault_contents: &'a [PathBuf],
frontmatter_strategy: FrontmatterStrategy, frontmatter_strategy: FrontmatterStrategy,
note_depth: u32,
} }
impl<'a> Context<'a> { impl<'a> Context<'a> {
/// Create a new `Context`
fn new(file: PathBuf, vault_contents: &'a [PathBuf]) -> Context<'a> { fn new(file: PathBuf, vault_contents: &'a [PathBuf]) -> Context<'a> {
Context { Context {
file, file_tree: vec![file.clone()],
vault_contents, vault_contents,
frontmatter_strategy: FrontmatterStrategy::Auto, frontmatter_strategy: FrontmatterStrategy::Auto,
note_depth: 1,
} }
} }
fn frontmatter_strategy(&mut self, strategy: FrontmatterStrategy) -> &mut Context<'a> { /// Create a new `Context` which inherits from a parent Context.
fn from_parent(context: &Context<'a>, child: &PathBuf) -> Context<'a> {
let mut context = context.clone();
context.file_tree.push(child.to_path_buf());
context
}
/// Associate a new `FrontmatterStrategy` with this context.
fn set_frontmatter_strategy(&mut self, strategy: FrontmatterStrategy) -> &mut Context<'a> {
self.frontmatter_strategy = strategy; self.frontmatter_strategy = strategy;
self self
} }
fn file(&mut self, file: PathBuf) -> &mut Context<'a> { /// Return the path of the file currently being parsed.
self.file = file; fn current_file(&self) -> &PathBuf {
self self.file_tree
.last()
.expect("Context not initialized properly, file_tree is empty")
} }
fn increment_depth(&mut self) -> &mut Context<'a> { /// Return the note depth (nesting level) for this context.
self.note_depth += 1; fn note_depth(&self) -> usize {
self self.file_tree.len()
}
/// Return the list of files associated with this context.
///
/// The first element corresponds to the root file, the final element corresponds to the file
/// which is currently being processed (see also `current_file`).
fn file_tree(&self) -> Vec<PathBuf> {
self.file_tree.clone()
} }
} }
@ -232,7 +249,7 @@ fn parse_and_export_obsidian_note(
} }
let mut context = Context::new(src.to_path_buf(), vault_contents); let mut context = Context::new(src.to_path_buf(), vault_contents);
context.frontmatter_strategy(frontmatter_strategy); context.set_frontmatter_strategy(frontmatter_strategy);
let markdown_tree = parse_obsidian_note(&src, &context)?; let markdown_tree = parse_obsidian_note(&src, &context)?;
outfile outfile
.write_all(render_mdtree_to_mdtext(markdown_tree).as_bytes()) .write_all(render_mdtree_to_mdtext(markdown_tree).as_bytes())
@ -241,9 +258,10 @@ fn parse_and_export_obsidian_note(
} }
fn parse_obsidian_note<'a>(path: &Path, context: &Context) -> Result<MarkdownTree<'a>> { fn parse_obsidian_note<'a>(path: &Path, context: &Context) -> Result<MarkdownTree<'a>> {
if context.note_depth > NOTE_RECURSION_LIMIT { if context.note_depth() > NOTE_RECURSION_LIMIT {
// TODO: Include parent so the source note can be traced back. return Err(ExportError::RecursionLimitExceeded {
return Err(ExportError::RecursionLimitExceeded {}); file_tree: context.file_tree(),
});
} }
let content = fs::read_to_string(&path).context(ReadError { path })?; let content = fs::read_to_string(&path).context(ReadError { path })?;
let (_frontmatter, content) = let (_frontmatter, content) =
@ -333,9 +351,7 @@ fn embed_file<'a, 'b>(note_name: &'a str, context: &'b Context) -> Result<Markdo
let tree = match lookup_filename_in_vault(note_name, context.vault_contents) { let tree = match lookup_filename_in_vault(note_name, context.vault_contents) {
Some(path) => { Some(path) => {
let mut context = context.clone(); let context = Context::from_parent(context, path);
context.file(path.to_path_buf()).increment_depth();
let no_ext = OsString::new(); let no_ext = OsString::new();
match path.extension().unwrap_or(&no_ext).to_str() { match path.extension().unwrap_or(&no_ext).to_str() {
Some("md") => parse_obsidian_note(&path, &context)?, Some("md") => parse_obsidian_note(&path, &context)?,
@ -373,7 +389,7 @@ fn embed_file<'a, 'b>(note_name: &'a str, context: &'b Context) -> Result<Markdo
println!( println!(
"Warning: Unable to find embedded note\n\tReference: '{}'\n\tSource: '{}'", "Warning: Unable to find embedded note\n\tReference: '{}'\n\tSource: '{}'",
note_name, note_name,
context.file.display(), context.current_file().display(),
); );
vec![] vec![]
} }
@ -400,7 +416,7 @@ fn make_link_to_file<'a>(file: &'a str, label: &'a str, context: &Context) -> Ma
println!( println!(
"Warning: Unable to find referenced note\n\tReference: '{}'\n\tSource: '{}'", "Warning: Unable to find referenced note\n\tReference: '{}'\n\tSource: '{}'",
file, file,
context.file.display(), context.current_file().display(),
); );
return vec![ return vec![
Event::Start(Tag::Emphasis), Event::Start(Tag::Emphasis),
@ -412,7 +428,7 @@ fn make_link_to_file<'a>(file: &'a str, label: &'a str, context: &Context) -> Ma
let rel_link = diff_paths( let rel_link = diff_paths(
target_file, target_file,
&context &context
.file .current_file()
.parent() .parent()
.expect("obsidian content files should always have a parent"), .expect("obsidian content files should always have a parent"),
) )

View File

@ -1,6 +1,6 @@
use eyre::{eyre, Result}; use eyre::{eyre, Result};
use gumdrop::Options; use gumdrop::Options;
use obsidian_export::{Exporter, FrontmatterStrategy}; use obsidian_export::{ExportError, Exporter, FrontmatterStrategy};
use std::path::PathBuf; use std::path::PathBuf;
#[derive(Debug, Options)] #[derive(Debug, Options)]
@ -42,7 +42,35 @@ fn main() -> Result<()> {
exporter.frontmatter_strategy(args.frontmatter_strategy); exporter.frontmatter_strategy(args.frontmatter_strategy);
// TODO: Pass in configurable walk_options here: exporter.walk_options(..); // TODO: Pass in configurable walk_options here: exporter.walk_options(..);
// TODO: This should allow settings for ignore_hidden and honor_gitignore. // TODO: This should allow settings for ignore_hidden and honor_gitignore.
exporter.run()?;
if let Err(err) = exporter.run() {
match err {
ExportError::FileExportError {
ref path,
ref source,
} => match &**source {
// An arguably better way of enhancing error reports would be to construct a custom
// `eyre::EyreHandler`, but that would require a fair amount of boilerplate and
// reimplementation of basic reporting.
ExportError::RecursionLimitExceeded { file_tree } => {
eprintln!(
"Error: {:?}",
eyre!(
"'{}' exceeds the maximum nesting limit of embeds",
path.display()
)
);
eprintln!("\nFile tree:");
for (idx, path) in file_tree.iter().enumerate() {
eprintln!("{}-> {}", " ".repeat(idx), path.display());
}
}
_ => eprintln!("Error: {:?}", eyre!(err)),
},
_ => eprintln!("Error: {:?}", eyre!(err)),
};
std::process::exit(1);
};
Ok(()) Ok(())
} }

View File

@ -266,7 +266,7 @@ fn test_infinite_recursion() {
match err { match err {
ExportError::FileExportError { path: _, source } => match *source { ExportError::FileExportError { path: _, source } => match *source {
ExportError::RecursionLimitExceeded {} => {} ExportError::RecursionLimitExceeded { .. } => {}
_ => panic!("Wrong error variant for source, got: {:?}", source), _ => panic!("Wrong error variant for source, got: {:?}", source),
}, },
err => panic!("Wrong error variant: {:?}", err), err => panic!("Wrong error variant: {:?}", err),