added an option to include more than one folder from the root #665
Conversation
zleyyij
left a comment
zleyyij
left a comment
There was a problem hiding this comment.
great work, a little bit confused as to why a whole bunch of typescript was commented out, if intentional plz leave comment explaining why
| } | ||
| } | ||
| if children.is_empty() { | ||
| Err("No doc tree found in any doc root".to_owned()) |
There was a problem hiding this comment.
The idiomatic way to do this with color_eyre is with the bail! macro, see https://docs.rs/eyre/latest/eyre/macro.bail.html
There was a problem hiding this comment.
Cannot use bail as it isn't compatible with <INode, String> restults
There was a problem hiding this comment.
I don't see a particular reason this function needs to return a string in the Err condition, is there anyway you could coerce any strings into color_eyre errors instead of coercing errors into strings?
| /// | ||
| /// EG: `./repo/docs` | ||
| doc_path: PathBuf, | ||
| doc_path: Vec<PathBuf>, |
There was a problem hiding this comment.
This was pluralized but the docstring was not, you might want to explain what multiple paths does and how it works/why you'd want multiple
There was a problem hiding this comment.
Asked in discord to explain further.
| return Ok(()); | ||
| } | ||
| } | ||
| Err(format!("Document {:?} not found in any doc root", path.as_ref())) |
There was a problem hiding this comment.
bail! again https://docs.rs/eyre/latest/eyre/macro.bail.html
There was a problem hiding this comment.
Same as before cannot use bail as it isn't compatible with results type
There was a problem hiding this comment.
This could definitely be a color_eyre::Result, then you could use bail!
…or now until updates occur.
…or now until updates occur.
| } | ||
| } | ||
| if children.is_empty() { | ||
| Err("No doc tree found in any doc root".to_owned()) |
There was a problem hiding this comment.
I don't see a particular reason this function needs to return a string in the Err condition, is there anyway you could coerce any strings into color_eyre errors instead of coercing errors into strings?
| return Ok(()); | ||
| } | ||
| } | ||
| Err(format!("Document {:?} not found in any doc root", path.as_ref())) |
There was a problem hiding this comment.
This could definitely be a color_eyre::Result, then you could use bail!
2 participants
This should allow you to add more than one folder from root, in case you have multiple doc paths, such as we do for the wiki.