From cfc616465302f3d6cf42a746ac458242c3e30882 Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Wed, 20 Mar 2024 18:22:07 +0100 Subject: [PATCH 1/9] init: frontmatter support --- Cargo.lock | 77 ++++++++++++++++++++++++++++++++++++- Cargo.toml | 2 + doc/frontmatter.md | 96 ++++++++++++++++++++++++++++++++++++++++++++++ src/frontmatter.rs | 60 +++++++++++++++++++++++++++++ src/main.rs | 11 +++++- 5 files changed, 243 insertions(+), 3 deletions(-) create mode 100644 doc/frontmatter.md create mode 100644 src/frontmatter.rs diff --git a/Cargo.lock b/Cargo.lock index 344f5a3..80cbef3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -126,6 +126,18 @@ version = "0.3.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a357d28ed41a50f9c765dbfe56cbc04a64e53e5fc58ba79fbc34c10ef3df831f" +[[package]] +name = "equivalent" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" + +[[package]] +name = "hashbrown" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" + [[package]] name = "hashbrown" version = "0.14.3" @@ -138,6 +150,26 @@ version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "95505c38b4572b2d910cecb0281560f54b440a19336cbbcb27bf6ce6adc6f5a8" +[[package]] +name = "indexmap" +version = "1.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bd070e393353796e801d209ad339e89596eb4c8d430d18ede6a1cced8fafbd99" +dependencies = [ + "autocfg", + "hashbrown 0.12.3", +] + +[[package]] +name = "indexmap" +version = "2.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7b0b929d511467233429c45a44ac1dcaa21ba0f5ba11e4879e6ed28ddb4f9df4" +dependencies = [ + "equivalent", + "hashbrown 0.14.3", +] + [[package]] name = "insta" version = "1.36.1" @@ -194,7 +226,9 @@ dependencies = [ "rowan", "serde", "serde_json", + "serde_yaml 0.9.33", "textwrap", + "yaml-front-matter", ] [[package]] @@ -231,7 +265,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32a58fa8a7ccff2aec4f39cc45bf5f985cec7125ab271cf681c279fd00192b49" dependencies = [ "countme", - "hashbrown", + "hashbrown 0.14.3", "memoffset", "rustc-hash", "text-size", @@ -280,6 +314,31 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_yaml" +version = "0.8.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "578a7433b776b56a35785ed5ce9a7e777ac0598aac5a6dd1b4b18a307c7fc71b" +dependencies = [ + "indexmap 1.9.3", + "ryu", + "serde", + "yaml-rust", +] + +[[package]] +name = "serde_yaml" +version = "0.9.33" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a0623d197252096520c6f2a5e1171ee436e5af99a5d7caa2891e55e61950e6d9" +dependencies = [ + "indexmap 2.2.5", + "itoa", + "ryu", + "serde", + "unsafe-libyaml", +] + [[package]] name = "similar" version = "2.4.0" @@ -344,6 +403,12 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e51733f11c9c4f72aa0c160008246859e340b00807569a0da0e7a1079b27ba85" +[[package]] +name = "unsafe-libyaml" +version = "0.2.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" + [[package]] name = "utf8parse" version = "0.2.1" @@ -416,6 +481,16 @@ version = "0.52.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32b752e52a2da0ddfbdbcc6fceadfeede4c939ed16d13e648833a61dfb611ed8" +[[package]] +name = "yaml-front-matter" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a94fb32d2b438e3fddf901fbfe9eb87b34d63853ca6c6da5d2ab7e27031e0bae" +dependencies = [ + "serde", + "serde_yaml 0.8.26", +] + [[package]] name = "yaml-rust" version = "0.4.5" diff --git a/Cargo.toml b/Cargo.toml index 1df6a8b..9e51288 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,6 +12,8 @@ serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" textwrap = "0.16" clap = { version = "4.4.4", features = ["derive"] } +serde_yaml = "0.9.33" +yaml-front-matter = "0.1.0" [dev-dependencies] insta = "1.36.1" diff --git a/doc/frontmatter.md b/doc/frontmatter.md new file mode 100644 index 0000000..cdd8876 --- /dev/null +++ b/doc/frontmatter.md @@ -0,0 +1,96 @@ +# Frontmatter + +This document is the specification for custom metadata within doc-comments. + +It should only apply to doc-comments (`/** */`) and not to regular code comments to ensure a limited scope. + +## Why frontmatter is needed (sometimes) + +Sometimes it is desireable to extend the native doc-comment functionality. For that user scenario, frontmatter can be optionally used. + +i.e., + +- Enriching documentation generation. +- Maintaining References. +- Metadata inclusion. +- Allow better Handling of edge cases. + +## Detailed design + +Fields (from Keywords list) can be defined in frontmatter. + +Frontmatter is defined using key-value pairs, encapsulated within triple-dashed lines (---). + +While there is no strict specification for frontmatter formats, YAML is commonly preferred. Although JSON could also be used alternatively. + +`{key}` is a placeholder for the list of available keywords listed below. + +`{value}` is a placeholder for the set value associated with the directive. + +Example: + +```nix +{ +/** + --- + import: ./path.md + --- +*/ +foo = x: x; +} +``` + +In this example, the `import` directive fetches content from `./path.md` treating it as if it were directly within the doc-comment. +This allows tracking the reference between the source position and the markdown file, in case of extensive documentation. + +## Keywords + +### Import + +Rules: + +1. The `import` keyword can be used to use content from another file INSTEAD of the doc-comments content. + +2. The value `file` must be given as an absolute path (relative to git root) or relative to the current file. + +3. There can only be one `import` per doc-comment. + +```nix +{ +/** + --- + import: ./path.md + --- +*/ +foo = x: x; +} +``` + +`path.md` +```md +some nice docs! +``` + +Rendering this would behave as if the content where actually placed in the doc-comment itself. + +Placing frontmatter inside the imported file will be ignored. (No nested directives) +Since handling recursive or nested imports adds too much complexity for little or no benefit. + +> Note: Absolute path imports are relative to the repository root. They only work inside `git` repositories and require having the `git` binary in PATH. +> This is most commonly used in large repositories or when the documentation files are not placed alongside the .nix files. + +## Extensibility + +The initial set of keywords is intentionally minimalistic, +focusing on immediate and broadly applicable needs. + +Community contributions are encouraged to expand this list as new use cases emerge. + +## Error handling + +Any issues encountered during the processing of frontmatter—be it syntax errors, invalid paths, or unsupported keywords—should result in clear, actionable error messages to the user. + +## Future work + +This proposal represents a foundational step towards more versatile and extendable reference documentation. +As we move forward, we'll remain open to adapting and expanding this specification to meet emerging needs and leverage community insights. diff --git a/src/frontmatter.rs b/src/frontmatter.rs new file mode 100644 index 0000000..33e9fd8 --- /dev/null +++ b/src/frontmatter.rs @@ -0,0 +1,60 @@ +use std::{collections::HashMap, fs, path::PathBuf, process::Command}; + +use serde::{Deserialize, Serialize}; +use yaml_front_matter::{Document, YamlFrontMatter}; + +fn find_repo_root() -> Option { + let output = Command::new("git") + .args(&["rev-parse", "--show-toplevel"]) + .output() + .ok()? + .stdout; + + let path_str = String::from_utf8(output).ok()?.trim().to_string(); + Some(PathBuf::from(path_str)) +} + +#[derive(Deserialize, Serialize, Debug)] +pub struct Matter { + #[serde(flatten)] + pub content: HashMap, +} + +/// Returns the actual content of a markdown file, if the frontmatter has an import field. +pub fn get_imported_content(file_path: &PathBuf, markdown: Option<&String>) -> Option { + if markdown.is_none() { + return None; + } + + match YamlFrontMatter::parse::(&markdown.unwrap()) { + Ok(document) => { + let metadata = document.metadata.content; + + let abs_import = metadata.get("import").map(|field| { + let import_val = field + .as_str() + .expect("Frontmatter: import field must be a string"); + match PathBuf::from(import_val).is_relative() { + true => PathBuf::from_iter(vec![ + // Cannot fail because every file has a parent directory + file_path.parent().unwrap().to_path_buf(), + PathBuf::from(import_val), + ]), + false => PathBuf::from_iter(vec![ + find_repo_root() + .expect("Could not find root directory of repository. Make sure you have git installed and are in a git repository"), + PathBuf::from(format!(".{import_val}")), + ]), + } + }); + + abs_import.map(|path| { + fs::read_to_string(&path) + .expect(format!("Could not read file: {:?}", &path).as_str()) + }) + } + Err(e) => { + return None; + } + } +} diff --git a/src/main.rs b/src/main.rs index 3a93330..275d4b3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -24,6 +24,7 @@ mod comment; mod commonmark; mod format; +mod frontmatter; mod legacy; #[cfg(test)] mod test; @@ -33,6 +34,7 @@ use crate::{format::handle_indentation, legacy::retrieve_legacy_comment}; use self::comment::get_expr_docs; use self::commonmark::*; use format::shift_headings; +use frontmatter::get_imported_content; use legacy::{collect_lambda_args_legacy, LegacyDocItem}; use rnix::{ ast::{Attr, AttrpathValue, Expr, Inherit, LetIn}, @@ -102,9 +104,14 @@ enum DocItemOrLegacy { pub fn retrieve_doc_comment(node: &SyntaxNode, shift_headings_by: Option) -> Option { let doc_comment = get_expr_docs(node); - doc_comment.map(|doc_comment| { + let opts = Options::parse(); + + // Doc comments can import external file via "import" in frontmatter + let content = get_imported_content(&opts.file, doc_comment.as_ref()).or(doc_comment); + + content.map(|inner| { shift_headings( - &handle_indentation(&doc_comment).unwrap_or(String::new()), + &handle_indentation(&inner).unwrap_or(String::new()), // H1 to H4 can be used in the doc-comment with the current rendering. // They will be shifted to H3, H6 // H1 and H2 are currently used by the outer rendering. (category and function name) From 917a8f69abfc3fe74373322042f886335789857f Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Wed, 20 Mar 2024 18:59:03 +0100 Subject: [PATCH 2/9] fix: opts.file must be piped through everywhere --- src/frontmatter.rs | 23 ++++++++++---------- src/legacy.rs | 6 ++++-- src/main.rs | 50 ++++++++++++++++++++++++++++---------------- src/test.rs | 52 +++++++++++++++++++++++++++------------------- 4 files changed, 79 insertions(+), 52 deletions(-) diff --git a/src/frontmatter.rs b/src/frontmatter.rs index 33e9fd8..03fdccc 100644 --- a/src/frontmatter.rs +++ b/src/frontmatter.rs @@ -1,11 +1,16 @@ -use std::{collections::HashMap, fs, path::PathBuf, process::Command}; +use std::{ + collections::HashMap, + fs, + path::{Path, PathBuf}, + process::Command, +}; use serde::{Deserialize, Serialize}; -use yaml_front_matter::{Document, YamlFrontMatter}; +use yaml_front_matter::YamlFrontMatter; fn find_repo_root() -> Option { let output = Command::new("git") - .args(&["rev-parse", "--show-toplevel"]) + .args(["rev-parse", "--show-toplevel"]) .output() .ok()? .stdout; @@ -21,12 +26,10 @@ pub struct Matter { } /// Returns the actual content of a markdown file, if the frontmatter has an import field. -pub fn get_imported_content(file_path: &PathBuf, markdown: Option<&String>) -> Option { - if markdown.is_none() { - return None; - } +pub fn get_imported_content(file_path: &Path, markdown: Option<&String>) -> Option { + markdown?; - match YamlFrontMatter::parse::(&markdown.unwrap()) { + match YamlFrontMatter::parse::(markdown.unwrap()) { Ok(document) => { let metadata = document.metadata.content; @@ -53,8 +56,6 @@ pub fn get_imported_content(file_path: &PathBuf, markdown: Option<&String>) -> O .expect(format!("Could not read file: {:?}", &path).as_str()) }) } - Err(e) => { - return None; - } + Err(_) => None, } } diff --git a/src/legacy.rs b/src/legacy.rs index fc11923..3aa873d 100644 --- a/src/legacy.rs +++ b/src/legacy.rs @@ -1,3 +1,5 @@ +use std::path::Path; + use rnix::{ ast::{AstToken, Comment, Expr, Lambda, Param}, SyntaxKind, SyntaxNode, @@ -96,7 +98,7 @@ pub fn retrieve_legacy_comment(node: &SyntaxNode, allow_line_comments: bool) -> /// Traverse directly chained nix lambdas and collect the identifiers of all lambda arguments /// until an unexpected AST node is encountered. -pub fn collect_lambda_args_legacy(mut lambda: Lambda) -> Vec { +pub fn collect_lambda_args_legacy(mut lambda: Lambda, file_path: &Path) -> Vec { let mut args = vec![]; loop { @@ -120,7 +122,7 @@ pub fn collect_lambda_args_legacy(mut lambda: Lambda) -> Vec { .map(|entry| SingleArg { name: entry.ident().unwrap().to_string(), doc: handle_indentation( - &retrieve_doc_comment(entry.syntax(), Some(1)) + &retrieve_doc_comment(entry.syntax(), Some(1), file_path) .or(retrieve_legacy_comment(entry.syntax(), true)) .unwrap_or_default(), ), diff --git a/src/main.rs b/src/main.rs index 275d4b3..ee8055e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -41,7 +41,7 @@ use rnix::{ SyntaxKind, SyntaxNode, }; use rowan::{ast::AstNode, WalkEvent}; -use std::fs; +use std::{fs, path::Path}; use std::collections::HashMap; use std::io; @@ -101,13 +101,15 @@ enum DocItemOrLegacy { } /// Returns a rfc145 doc-comment if one is present -pub fn retrieve_doc_comment(node: &SyntaxNode, shift_headings_by: Option) -> Option { +pub fn retrieve_doc_comment( + node: &SyntaxNode, + shift_headings_by: Option, + file: &Path, +) -> Option { let doc_comment = get_expr_docs(node); - let opts = Options::parse(); - // Doc comments can import external file via "import" in frontmatter - let content = get_imported_content(&opts.file, doc_comment.as_ref()).or(doc_comment); + let content = get_imported_content(file, doc_comment.as_ref()).or(doc_comment); content.map(|inner| { shift_headings( @@ -122,14 +124,14 @@ pub fn retrieve_doc_comment(node: &SyntaxNode, shift_headings_by: Option) /// Transforms an AST node into a `DocItem` if it has a leading /// documentation comment. -fn retrieve_doc_item(node: &AttrpathValue) -> Option { +fn retrieve_doc_item(node: &AttrpathValue, file_path: &Path) -> Option { let ident = node.attrpath().unwrap(); // TODO this should join attrs() with '.' to handle whitespace, dynamic attrs and string // attrs. none of these happen in nixpkgs lib, and the latter two should probably be // rejected entirely. let item_name = ident.to_string(); - let doc_comment = retrieve_doc_comment(node.syntax(), Some(2)); + let doc_comment = retrieve_doc_comment(node.syntax(), Some(2), file_path); match doc_comment { Some(comment) => Some(DocItemOrLegacy::DocItem(DocItem { name: item_name, @@ -199,14 +201,14 @@ fn parse_doc_comment(raw: &str) -> DocComment { /// 2. The attached doc comment on the entry. /// 3. The argument names of any curried functions (pattern functions /// not yet supported). -fn collect_entry_information(entry: AttrpathValue) -> Option { - let doc_item = retrieve_doc_item(&entry)?; +fn collect_entry_information(entry: AttrpathValue, file_path: &Path) -> Option { + let doc_item = retrieve_doc_item(&entry, file_path)?; match doc_item { DocItemOrLegacy::LegacyDocItem(v) => { if let Some(Expr::Lambda(l)) = entry.value() { Some(LegacyDocItem { - args: collect_lambda_args_legacy(l), + args: collect_lambda_args_legacy(l, file_path), ..v }) } else { @@ -230,6 +232,7 @@ fn collect_bindings( prefix: &str, category: &str, scope: HashMap, + file_path: &Path, ) -> Vec { for ev in node.preorder() { match ev { @@ -238,7 +241,7 @@ fn collect_bindings( for child in n.children() { if let Some(apv) = AttrpathValue::cast(child.clone()) { entries.extend( - collect_entry_information(apv) + collect_entry_information(apv, file_path) .map(|di| di.into_entry(prefix, category)), ); } else if let Some(inh) = Inherit::cast(child) { @@ -266,7 +269,12 @@ fn collect_bindings( // Main entrypoint for collection // TODO: document -fn collect_entries(root: rnix::Root, prefix: &str, category: &str) -> Vec { +fn collect_entries( + root: rnix::Root, + prefix: &str, + category: &str, + file_path: &Path, +) -> Vec { // we will look into the top-level let and its body for function docs. // we only need a single level of scope for this. // since only the body can export a function we don't need to implement @@ -280,13 +288,14 @@ fn collect_entries(root: rnix::Root, prefix: &str, category: &str) -> Vec { - return collect_bindings(&n, prefix, category, Default::default()); + return collect_bindings(&n, prefix, category, Default::default(), file_path); } _ => (), } @@ -295,14 +304,19 @@ fn collect_entries(root: rnix::Root, prefix: &str, category: &str) -> Vec String { +fn retrieve_description( + nix: &rnix::Root, + description: &str, + category: &str, + file: &Path, +) -> String { format!( "# {} {{#sec-functions-library-{}}}\n{}\n", description, category, &nix.syntax() .first_child() - .and_then(|node| retrieve_doc_comment(&node, Some(1)) + .and_then(|node| retrieve_doc_comment(&node, Some(1), file) .or(retrieve_legacy_comment(&node, false))) .and_then(|doc_item| handle_indentation(&doc_item)) .unwrap_or_default() @@ -321,12 +335,12 @@ fn main() { .expect("could not read location information"), }; let nix = rnix::Root::parse(&src).ok().expect("failed to parse input"); - let description = retrieve_description(&nix, &opts.description, &opts.category); + let description = retrieve_description(&nix, &opts.description, &opts.category, &opts.file); // TODO: move this to commonmark.rs writeln!(output, "{}", description).expect("Failed to write header"); - for entry in collect_entries(nix, &opts.prefix, &opts.category) { + for entry in collect_entries(nix, &opts.prefix, &opts.category, &opts.file) { entry .write_section(&locs, &mut output) .expect("Failed to write section") diff --git a/src/test.rs b/src/test.rs index cafd651..96e2a26 100644 --- a/src/test.rs +++ b/src/test.rs @@ -1,5 +1,6 @@ use rnix; use std::fs; +use std::path::PathBuf; use std::io::Write; @@ -8,7 +9,8 @@ use crate::{collect_entries, format::shift_headings, retrieve_description}; #[test] fn test_main() { let mut output = Vec::new(); - let src = fs::read_to_string("test/strings.nix").unwrap(); + let src_path = PathBuf::from("test/strings.nix"); + let src = fs::read_to_string(&src_path).unwrap(); let locs = serde_json::from_str(&fs::read_to_string("test/strings.json").unwrap()).unwrap(); let nix = rnix::Root::parse(&src).ok().expect("failed to parse input"); let desc = "string manipulation functions"; @@ -23,7 +25,7 @@ fn test_main() { ) .expect("Failed to write header"); - for entry in collect_entries(nix, prefix, category) { + for entry in collect_entries(nix, prefix, category, &src_path) { entry .write_section(&locs, &mut output) .expect("Failed to write section") @@ -37,14 +39,15 @@ fn test_main() { #[test] fn test_description_of_lib_debug() { let mut output = Vec::new(); - let src = fs::read_to_string("test/lib-debug.nix").unwrap(); + let src_path = PathBuf::from("test/lib-debug.nix"); + let src = fs::read_to_string(&src_path).unwrap(); let nix = rnix::Root::parse(&src).ok().expect("failed to parse input"); let prefix = "lib"; let category = "debug"; - let desc = retrieve_description(&nix, &"Debug", category); + let desc = retrieve_description(&nix, &"Debug", category, &src_path); writeln!(output, "{}", desc).expect("Failed to write header"); - for entry in collect_entries(nix, prefix, category) { + for entry in collect_entries(nix, prefix, category, &src_path) { entry .write_section(&Default::default(), &mut output) .expect("Failed to write section") @@ -58,12 +61,13 @@ fn test_description_of_lib_debug() { #[test] fn test_arg_formatting() { let mut output = Vec::new(); - let src = fs::read_to_string("test/arg-formatting.nix").unwrap(); + let src_path = PathBuf::from("test/arg-formatting.nix"); + let src = fs::read_to_string(&src_path).unwrap(); let nix = rnix::Root::parse(&src).ok().expect("failed to parse input"); let prefix = "lib"; let category = "options"; - for entry in collect_entries(nix, prefix, category) { + for entry in collect_entries(nix, prefix, category, &src_path) { entry .write_section(&Default::default(), &mut output) .expect("Failed to write section") @@ -77,12 +81,13 @@ fn test_arg_formatting() { #[test] fn test_inherited_exports() { let mut output = Vec::new(); - let src = fs::read_to_string("test/inherited-exports.nix").unwrap(); + let src_path = PathBuf::from("test/inherited-exports.nix"); + let src = fs::read_to_string(&src_path).unwrap(); let nix = rnix::Root::parse(&src).ok().expect("failed to parse input"); let prefix = "lib"; let category = "let"; - for entry in collect_entries(nix, prefix, category) { + for entry in collect_entries(nix, prefix, category, &src_path) { entry .write_section(&Default::default(), &mut output) .expect("Failed to write section") @@ -96,12 +101,13 @@ fn test_inherited_exports() { #[test] fn test_line_comments() { let mut output = Vec::new(); - let src = fs::read_to_string("test/line-comments.nix").unwrap(); + let src_path = PathBuf::from("test/line-comments.nix"); + let src = fs::read_to_string(&src_path).unwrap(); let nix = rnix::Root::parse(&src).ok().expect("failed to parse input"); let prefix = "lib"; let category = "let"; - for entry in collect_entries(nix, prefix, category) { + for entry in collect_entries(nix, prefix, category, &src_path) { entry .write_section(&Default::default(), &mut output) .expect("Failed to write section") @@ -115,12 +121,13 @@ fn test_line_comments() { #[test] fn test_multi_line() { let mut output = Vec::new(); - let src = fs::read_to_string("test/multi-line.nix").unwrap(); + let src_path = PathBuf::from("test/multi-line.nix"); + let src = fs::read_to_string(&src_path).unwrap(); let nix = rnix::Root::parse(&src).ok().expect("failed to parse input"); let prefix = "lib"; let category = "let"; - for entry in collect_entries(nix, prefix, category) { + for entry in collect_entries(nix, prefix, category, &src_path) { entry .write_section(&Default::default(), &mut output) .expect("Failed to write section") @@ -134,12 +141,13 @@ fn test_multi_line() { #[test] fn test_doc_comment() { let mut output = Vec::new(); - let src = fs::read_to_string("test/doc-comment.nix").unwrap(); + let src_path = PathBuf::from("test/doc-comment.nix"); + let src = fs::read_to_string(&src_path).unwrap(); let nix = rnix::Root::parse(&src).ok().expect("failed to parse input"); let prefix = "lib"; let category = "debug"; - for entry in collect_entries(nix, prefix, category) { + for entry in collect_entries(nix, prefix, category, &src_path) { entry .write_section(&Default::default(), &mut output) .expect("Failed to write section") @@ -162,14 +170,15 @@ fn test_headings() { #[test] fn test_doc_comment_section_description() { let mut output = Vec::new(); - let src = fs::read_to_string("test/doc-comment-sec-heading.nix").unwrap(); + let src_path = PathBuf::from("test/doc-comment-sec-heading.nix"); + let src = fs::read_to_string(&src_path).unwrap(); let nix = rnix::Root::parse(&src).ok().expect("failed to parse input"); let prefix = "lib"; let category = "debug"; - let desc = retrieve_description(&nix, &"Debug", category); + let desc = retrieve_description(&nix, &"Debug", category, &src_path); writeln!(output, "{}", desc).expect("Failed to write header"); - for entry in collect_entries(nix, prefix, category) { + for entry in collect_entries(nix, prefix, category, &src_path) { entry .write_section(&Default::default(), &mut output) .expect("Failed to write section") @@ -183,14 +192,15 @@ fn test_doc_comment_section_description() { #[test] fn test_doc_comment_no_duplicate_arguments() { let mut output = Vec::new(); - let src = fs::read_to_string("test/doc-comment-arguments.nix").unwrap(); + let src_path = PathBuf::from("test/doc-comment-arguments.nix"); + let src = fs::read_to_string(&src_path).unwrap(); let nix = rnix::Root::parse(&src).ok().expect("failed to parse input"); let prefix = "lib"; let category = "debug"; - let desc = retrieve_description(&nix, &"Debug", category); + let desc = retrieve_description(&nix, &"Debug", category, &src_path); writeln!(output, "{}", desc).expect("Failed to write header"); - for entry in collect_entries(nix, prefix, category) { + for entry in collect_entries(nix, prefix, category, &src_path) { entry .write_section(&Default::default(), &mut output) .expect("Failed to write section") From 71d849ed57cf4711473f84768701f2ad97dd500b Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Fri, 22 Mar 2024 21:29:19 +0100 Subject: [PATCH 3/9] Apply suggestions from code review Co-authored-by: Daniel Sidhion --- doc/frontmatter.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/frontmatter.md b/doc/frontmatter.md index cdd8876..546e284 100644 --- a/doc/frontmatter.md +++ b/doc/frontmatter.md @@ -21,7 +21,8 @@ Fields (from Keywords list) can be defined in frontmatter. Frontmatter is defined using key-value pairs, encapsulated within triple-dashed lines (---). -While there is no strict specification for frontmatter formats, YAML is commonly preferred. Although JSON could also be used alternatively. +While there is no strict specification for frontmatter formats, YAML is commonly preferred. +Although JSON could also be used alternatively. `{key}` is a placeholder for the list of available keywords listed below. @@ -81,8 +82,7 @@ Since handling recursive or nested imports adds too much complexity for little o ## Extensibility -The initial set of keywords is intentionally minimalistic, -focusing on immediate and broadly applicable needs. +The initial set of keywords is intentionally minimalistic, focusing on immediate and broadly applicable needs. Community contributions are encouraged to expand this list as new use cases emerge. From c48fb33166807de9e6e3e4c9af5191f49878770c Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Fri, 22 Mar 2024 21:46:47 +0100 Subject: [PATCH 4/9] doc: improve design document --- doc/frontmatter.md | 64 ++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 33 deletions(-) diff --git a/doc/frontmatter.md b/doc/frontmatter.md index 546e284..7a8adb6 100644 --- a/doc/frontmatter.md +++ b/doc/frontmatter.md @@ -6,64 +6,67 @@ It should only apply to doc-comments (`/** */`) and not to regular code comments ## Why frontmatter is needed (sometimes) -Sometimes it is desireable to extend the native doc-comment functionality. For that user scenario, frontmatter can be optionally used. +Sometimes it is desireable to extend the native doc-comment functionality. For that user-scenario, frontmatter can be optionally used. -i.e., +Frontmatter is the de-facto standard of adding document specific meta tags to markdown. [1](https://docs.github.com/en/contributing/writing-for-github-docs/using-yaml-frontmatter) [2](https://jekyllrb.com/docs/front-matter/) [3](https://starlight.astro.build/reference/frontmatter/) + +it can be useful for: - Enriching documentation generation. - Maintaining References. - Metadata inclusion. -- Allow better Handling of edge cases. +- Allow better Handling of documentation edge cases. ## Detailed design -Fields (from Keywords list) can be defined in frontmatter. - -Frontmatter is defined using key-value pairs, encapsulated within triple-dashed lines (---). +Frontmatter is defined using `key`-`value` pairs, encapsulated within triple-dashed lines (---). While there is no strict specification for frontmatter formats, YAML is commonly preferred. Although JSON could also be used alternatively. -`{key}` is a placeholder for the list of available keywords listed below. +Only `key`s from the list of available keywords can be used. -`{value}` is a placeholder for the set value associated with the directive. +The `value` is any valid yaml value. Its type and semantics is specified by each keyword individually. Example: ```nix { -/** - --- - import: ./path.md - --- -*/ -foo = x: x; + /** + --- + tags: + - foo + - bar + --- + */ + foo = x: x; } ``` -In this example, the `import` directive fetches content from `./path.md` treating it as if it were directly within the doc-comment. -This allows tracking the reference between the source position and the markdown file, in case of extensive documentation. +In this example `key` is `tags` and `value` is `List("foo", "bar")`. + +See also: [yaml specification](https://yaml.org/spec/1.2.2/) for how to use YAML. ## Keywords -### Import +### `doc-location` Rules: -1. The `import` keyword can be used to use content from another file INSTEAD of the doc-comments content. +1. The `doc-location` keyword can be used to use content from another file INSTEAD of the doc-comments content. -2. The value `file` must be given as an absolute path (relative to git root) or relative to the current file. +2. The value `file` must be given as a path relative to the current file. -3. There can only be one `import` per doc-comment. +3. Using this directive does not process any content inside the file, using it as-is. ```nix { -/** - --- - import: ./path.md - --- -*/ -foo = x: x; + /** + --- + doc-location: ./path.md + --- + */ + foo = x: x; } ``` @@ -72,13 +75,8 @@ foo = x: x; some nice docs! ``` -Rendering this would behave as if the content where actually placed in the doc-comment itself. - -Placing frontmatter inside the imported file will be ignored. (No nested directives) -Since handling recursive or nested imports adds too much complexity for little or no benefit. - -> Note: Absolute path imports are relative to the repository root. They only work inside `git` repositories and require having the `git` binary in PATH. -> This is most commonly used in large repositories or when the documentation files are not placed alongside the .nix files. +In this example, the `doc-location` directive fetches content from `./path.md` treating it as if it were directly within the doc-comment. +This allows tracking the reference between the source position and the markdown file, in case of external documentation. ## Extensibility From 6217f2e852117f5b9fef199246247d23fa79a870 Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Sat, 23 Mar 2024 00:30:53 +0100 Subject: [PATCH 5/9] unit tests: add frontmatter unit tests --- Cargo.lock | 70 ++++----- Cargo.toml | 3 +- src/frontmatter.rs | 134 ++++++++++++------ src/main.rs | 24 +++- ...c__test__frontmatter_doc_location_e2e.snap | 18 +++ src/test.rs | 80 ++++++++++- test/docs.md | 3 + test/frontmatter-doc-location.nix | 18 +++ 8 files changed, 260 insertions(+), 90 deletions(-) create mode 100644 src/snapshots/nixdoc__test__frontmatter_doc_location_e2e.snap create mode 100644 test/docs.md create mode 100644 test/frontmatter-doc-location.nix diff --git a/Cargo.lock b/Cargo.lock index 80cbef3..65f95b7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -133,10 +133,16 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" [[package]] -name = "hashbrown" -version = "0.12.3" +name = "gray_matter" +version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a9ee70c43aaf417c914396645a0fa852624801b24ebb7ae78fe8272889ac888" +checksum = "1cf2fb99fac0b821a4e61c61abff076324bb0e5c3b4a83815bbc3518a38971ad" +dependencies = [ + "serde", + "serde_json", + "toml", + "yaml-rust", +] [[package]] name = "hashbrown" @@ -150,16 +156,6 @@ version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "95505c38b4572b2d910cecb0281560f54b440a19336cbbcb27bf6ce6adc6f5a8" -[[package]] -name = "indexmap" -version = "1.9.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bd070e393353796e801d209ad339e89596eb4c8d430d18ede6a1cced8fafbd99" -dependencies = [ - "autocfg", - "hashbrown 0.12.3", -] - [[package]] name = "indexmap" version = "2.2.5" @@ -167,7 +163,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7b0b929d511467233429c45a44ac1dcaa21ba0f5ba11e4879e6ed28ddb4f9df4" dependencies = [ "equivalent", - "hashbrown 0.14.3", + "hashbrown", ] [[package]] @@ -221,14 +217,15 @@ name = "nixdoc" version = "3.0.2" dependencies = [ "clap", + "gray_matter", "insta", + "relative-path", "rnix", "rowan", "serde", "serde_json", - "serde_yaml 0.9.33", + "serde_yaml", "textwrap", - "yaml-front-matter", ] [[package]] @@ -249,6 +246,12 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "relative-path" +version = "1.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e898588f33fdd5b9420719948f9f2a32c922a246964576f71ba7f24f80610fbc" + [[package]] name = "rnix" version = "0.11.0" @@ -265,7 +268,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32a58fa8a7ccff2aec4f39cc45bf5f985cec7125ab271cf681c279fd00192b49" dependencies = [ "countme", - "hashbrown 0.14.3", + "hashbrown", "memoffset", "rustc-hash", "text-size", @@ -314,25 +317,13 @@ dependencies = [ "serde", ] -[[package]] -name = "serde_yaml" -version = "0.8.26" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "578a7433b776b56a35785ed5ce9a7e777ac0598aac5a6dd1b4b18a307c7fc71b" -dependencies = [ - "indexmap 1.9.3", - "ryu", - "serde", - "yaml-rust", -] - [[package]] name = "serde_yaml" version = "0.9.33" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a0623d197252096520c6f2a5e1171ee436e5af99a5d7caa2891e55e61950e6d9" dependencies = [ - "indexmap 2.2.5", + "indexmap", "itoa", "ryu", "serde", @@ -385,6 +376,15 @@ dependencies = [ "unicode-width", ] +[[package]] +name = "toml" +version = "0.5.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f4f7f0dd8d50a853a531c426359045b1998f04219d88799810762cd4ad314234" +dependencies = [ + "serde", +] + [[package]] name = "unicode-ident" version = "1.0.12" @@ -481,16 +481,6 @@ version = "0.52.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32b752e52a2da0ddfbdbcc6fceadfeede4c939ed16d13e648833a61dfb611ed8" -[[package]] -name = "yaml-front-matter" -version = "0.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a94fb32d2b438e3fddf901fbfe9eb87b34d63853ca6c6da5d2ab7e27031e0bae" -dependencies = [ - "serde", - "serde_yaml 0.8.26", -] - [[package]] name = "yaml-rust" version = "0.4.5" diff --git a/Cargo.toml b/Cargo.toml index 9e51288..9d383dd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,8 @@ serde_json = "1.0" textwrap = "0.16" clap = { version = "4.4.4", features = ["derive"] } serde_yaml = "0.9.33" -yaml-front-matter = "0.1.0" +gray_matter = "0.2.6" +relative-path = "1.9.2" [dev-dependencies] insta = "1.36.1" diff --git a/src/frontmatter.rs b/src/frontmatter.rs index 03fdccc..c9ce91b 100644 --- a/src/frontmatter.rs +++ b/src/frontmatter.rs @@ -1,61 +1,109 @@ use std::{ - collections::HashMap, fs, path::{Path, PathBuf}, - process::Command, }; +use gray_matter::engine::YAML; +use gray_matter::Matter; +use std::fmt; + +use relative_path::RelativePath; use serde::{Deserialize, Serialize}; -use yaml_front_matter::YamlFrontMatter; -fn find_repo_root() -> Option { - let output = Command::new("git") - .args(["rev-parse", "--show-toplevel"]) - .output() - .ok()? - .stdout; +#[derive(Deserialize, Serialize, Debug)] +pub struct Frontmatter { + pub doc_location: Option, +} - let path_str = String::from_utf8(output).ok()?.trim().to_string(); - Some(PathBuf::from(path_str)) +#[derive(Debug, PartialEq)] +pub enum FrontmatterErrorKind { + InvalidYaml, + DocLocationFileNotFound, + DocLocationNotRelativePath, } -#[derive(Deserialize, Serialize, Debug)] -pub struct Matter { - #[serde(flatten)] - pub content: HashMap, +#[derive(Debug, PartialEq)] +pub struct FrontmatterError { + pub message: String, + pub kind: FrontmatterErrorKind, +} + +impl fmt::Display for FrontmatterError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // Write the error message to the formatter + write!(f, "FrontmatterError: {}", self.message) + } } -/// Returns the actual content of a markdown file, if the frontmatter has an import field. -pub fn get_imported_content(file_path: &Path, markdown: Option<&String>) -> Option { - markdown?; - - match YamlFrontMatter::parse::(markdown.unwrap()) { - Ok(document) => { - let metadata = document.metadata.content; - - let abs_import = metadata.get("import").map(|field| { - let import_val = field - .as_str() - .expect("Frontmatter: import field must be a string"); - match PathBuf::from(import_val).is_relative() { - true => PathBuf::from_iter(vec![ - // Cannot fail because every file has a parent directory - file_path.parent().unwrap().to_path_buf(), - PathBuf::from(import_val), - ]), - false => PathBuf::from_iter(vec![ - find_repo_root() - .expect("Could not find root directory of repository. Make sure you have git installed and are in a git repository"), - PathBuf::from(format!(".{import_val}")), - ]), +/// Returns the actual content of a markdown file, if the frontmatter has a doc_location field. +/// It returns None if the frontmatter is not present. +/// It returns an error if the frontmatter is present but invalid. This includes: +/// - Invalid yaml frontmatter +/// - Invalid doc_location type +/// - doc_location file is not readable or not found +/// - doc_location field is not a relative path +/// - doc_location file is not utf8 +pub fn get_imported_content( + file_path: &Path, + markdown: &str, +) -> Result, FrontmatterError> { + let matter = Matter::::new(); + + let result = matter.parse(markdown); + + // If the frontmatter is not present, we return None + if result.data.is_none() { + return Ok(None); + } + + let pod = result.data.unwrap(); + match pod.deserialize::() { + Ok(metadata) => { + let abs_import = match metadata.doc_location { + Some(doc_location) => { + let import_path: PathBuf = PathBuf::from(&doc_location); + let relative_path = RelativePath::from_path(&import_path); + + match relative_path { + Ok(rel) => Ok(Some(rel.to_path(file_path.parent().unwrap()))), + Err(e) => Err(FrontmatterError { + message: format!("{:?}: doc_location: field must be a path relative to the current file. Error: {} - {}", file_path, doc_location, e), + kind: FrontmatterErrorKind::DocLocationNotRelativePath, + }), + } } - }); + // doc_location: field doesn't exist. Since it is optional, we return None + None => Ok(None), + }; + + match abs_import { + Ok(Some(path)) => match fs::read_to_string(&path) { + Ok(content) => Ok(Some(content)), + Err(e) => Err(FrontmatterError { + message: format!( + "{:?}: Failed to read doc_location file: {:?} {}", + file_path, path, e + ), + kind: FrontmatterErrorKind::DocLocationFileNotFound, + }), + }, + Ok(None) => Ok(None), + Err(e) => Err(e), + } + } - abs_import.map(|path| { - fs::read_to_string(&path) - .expect(format!("Could not read file: {:?}", &path).as_str()) + Err(e) => { + let message = format!( + "{:?}: Failed to parse frontmatter metadata - {} YAML:{}:{}", + file_path, + e, + e.line(), + e.column() + ); + Err(FrontmatterError { + message, + kind: FrontmatterErrorKind::InvalidYaml, }) } - Err(_) => None, } } diff --git a/src/main.rs b/src/main.rs index ee8055e..d3a8db6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -41,7 +41,7 @@ use rnix::{ SyntaxKind, SyntaxNode, }; use rowan::{ast::AstNode, WalkEvent}; -use std::{fs, path::Path}; +use std::{fs, path::Path, process::exit}; use std::collections::HashMap; use std::io; @@ -108,12 +108,26 @@ pub fn retrieve_doc_comment( ) -> Option { let doc_comment = get_expr_docs(node); - // Doc comments can import external file via "import" in frontmatter - let content = get_imported_content(file, doc_comment.as_ref()).or(doc_comment); + doc_comment.map(|inner| { + // Must handle indentation before processing yaml frontmatter + let content = handle_indentation(&inner).unwrap_or_default(); + + let final_content = match get_imported_content(file, &content) { + // Use the imported content instead of the original content + Ok(Some(imported_content)) => imported_content, + + // Use the original content + Ok(None) => content, + + // Abort if we failed to read the frontmatter + Err(e) => { + eprintln!("{}", e); + exit(1); + } + }; - content.map(|inner| { shift_headings( - &handle_indentation(&inner).unwrap_or(String::new()), + &handle_indentation(&final_content).unwrap_or(String::new()), // H1 to H4 can be used in the doc-comment with the current rendering. // They will be shifted to H3, H6 // H1 and H2 are currently used by the outer rendering. (category and function name) diff --git a/src/snapshots/nixdoc__test__frontmatter_doc_location_e2e.snap b/src/snapshots/nixdoc__test__frontmatter_doc_location_e2e.snap new file mode 100644 index 0000000..eeaa068 --- /dev/null +++ b/src/snapshots/nixdoc__test__frontmatter_doc_location_e2e.snap @@ -0,0 +1,18 @@ +--- +source: src/test.rs +expression: output +--- +# Debug {#sec-functions-library-debug} +## Imported + +This is be the documentation + +## `lib.debug.item` {#function-library-lib.debug.item} + +### Imported + +This is be the documentation + +## `lib.debug.optional` {#function-library-lib.debug.optional} + +No frontmatter diff --git a/src/test.rs b/src/test.rs index 96e2a26..fd89475 100644 --- a/src/test.rs +++ b/src/test.rs @@ -4,7 +4,12 @@ use std::path::PathBuf; use std::io::Write; -use crate::{collect_entries, format::shift_headings, retrieve_description}; +use crate::{ + collect_entries, + format::shift_headings, + frontmatter::{get_imported_content, FrontmatterError, FrontmatterErrorKind}, + retrieve_description, +}; #[test] fn test_main() { @@ -210,3 +215,76 @@ fn test_doc_comment_no_duplicate_arguments() { insta::assert_snapshot!(output); } + +#[test] +fn test_frontmatter_doc_location_e2e() { + let mut output = Vec::new(); + let src_path = PathBuf::from("test/frontmatter-doc-location.nix"); + let src = fs::read_to_string(&src_path).unwrap(); + let nix = rnix::Root::parse(&src).ok().expect("failed to parse input"); + let prefix = "lib"; + let category = "debug"; + let desc = retrieve_description(&nix, &"Debug", category, &src_path); + writeln!(output, "{}", desc).expect("Failed to write header"); + + for entry in collect_entries(nix, prefix, category, &src_path) { + entry + .write_section(&Default::default(), &mut output) + .expect("Failed to write section") + } + + let output = String::from_utf8(output).expect("not utf8"); + + insta::assert_snapshot!(output); +} + +const NOT_RELATIVE: &str = r#"--- +doc_location: /tmp/not-relative.md +--- +Other stuff +"#; + +#[test] +fn test_frontmatter_doc_location_relative() { + let base_file = PathBuf::from("test/frontmatter-doc-location.nix"); + + let result = get_imported_content(&base_file, NOT_RELATIVE); + + assert_eq!( + result.unwrap_err().kind, + FrontmatterErrorKind::DocLocationNotRelativePath + ); +} + +const INVALID_TYPE: &str = r#"--- +doc_location: 1 +--- +Other stuff +"#; + +#[test] +fn test_frontmatter_doc_location_type() { + let base_file = PathBuf::from("test/frontmatter-doc-location.nix"); + + let result = get_imported_content(&base_file, INVALID_TYPE); + + assert_eq!(result.unwrap_err().kind, FrontmatterErrorKind::InvalidYaml); +} + +const FILE_NOT_FOUND: &str = r#"--- +doc_location: ./does-not-exist.md +--- +Other stuff +"#; + +#[test] +fn test_frontmatter_doc_location_file_not_found() { + let base_file = PathBuf::from("test/frontmatter-doc-location.nix"); + + let result = get_imported_content(&base_file, FILE_NOT_FOUND); + + assert_eq!( + result.unwrap_err().kind, + FrontmatterErrorKind::DocLocationFileNotFound + ); +} diff --git a/test/docs.md b/test/docs.md new file mode 100644 index 0000000..99a1a67 --- /dev/null +++ b/test/docs.md @@ -0,0 +1,3 @@ +# Imported + +This is be the documentation diff --git a/test/frontmatter-doc-location.nix b/test/frontmatter-doc-location.nix new file mode 100644 index 0000000..a4d4c15 --- /dev/null +++ b/test/frontmatter-doc-location.nix @@ -0,0 +1,18 @@ +/** + --- + doc_location: docs.md + --- +*/ +{}: { + /** + --- + doc_location: docs.md + --- + */ + item = x: x; + + /** + No frontmatter + */ + optional = x: x; +} \ No newline at end of file From 47b1aa1b4503f1d8231b0e6d15429a31fa767dc9 Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Sat, 23 Mar 2024 00:34:30 +0100 Subject: [PATCH 6/9] rename doc-location -> doc_location --- doc/frontmatter.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/frontmatter.md b/doc/frontmatter.md index 7a8adb6..510cd13 100644 --- a/doc/frontmatter.md +++ b/doc/frontmatter.md @@ -49,13 +49,13 @@ See also: [yaml specification](https://yaml.org/spec/1.2.2/) for how to use YAML ## Keywords -### `doc-location` +### `doc_location` Rules: -1. The `doc-location` keyword can be used to use content from another file INSTEAD of the doc-comments content. +1. The `doc_location` keyword can be used to use content from another file INSTEAD of the doc-comments content. -2. The value `file` must be given as a path relative to the current file. +2. The value must be given as a path relative to the current file. 3. Using this directive does not process any content inside the file, using it as-is. @@ -63,7 +63,7 @@ Rules: { /** --- - doc-location: ./path.md + doc_location: ./path.md --- */ foo = x: x; @@ -75,7 +75,7 @@ Rules: some nice docs! ``` -In this example, the `doc-location` directive fetches content from `./path.md` treating it as if it were directly within the doc-comment. +In this example, the `doc_location` directive fetches content from `./path.md` treating it as if it were directly within the doc-comment. This allows tracking the reference between the source position and the markdown file, in case of external documentation. ## Extensibility From 6267d6a885056520446c2b9aa140e342f8dd1703 Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Sat, 23 Mar 2024 10:36:59 +0100 Subject: [PATCH 7/9] frontammter: bump release version --- CHANGELOG.md | 12 ++++++++++++ Cargo.lock | 2 +- Cargo.toml | 4 ++-- README.md | 44 ++++++++++++++++++++++++++++++++++++++------ doc/frontmatter.md | 34 ++++++++++++++++++++++++++++++---- 5 files changed, 83 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f8aeb8..8bf233a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Changelog +## Version 3.1.0 + +Added frontmatter feature: + +- `doc_location` keyword allows to externalize documentation. + +See our [full specification](./doc/frontmatter.md). + +by @hsjobeki; + +in https://github.com/nix-community/nixdoc/pull/114. + ## Version 3.0.2 Avoid displaying arguments when a doc-comment is already in place. diff --git a/Cargo.lock b/Cargo.lock index 65f95b7..0f9b844 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -214,7 +214,7 @@ dependencies = [ [[package]] name = "nixdoc" -version = "3.0.2" +version = "3.1.0" dependencies = [ "clap", "gray_matter", diff --git a/Cargo.toml b/Cargo.toml index 9d383dd..b62a3a1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "nixdoc" -version = "3.0.2" -authors = ["Vincent Ambo ", "asymmetric"] +version = "3.1.0" +authors = ["Vincent Ambo ", "asymmetric", "hsjobeki"] edition = "2021" description = "Generate CommonMark from Nix library functions" diff --git a/README.md b/README.md index e24b741..393c622 100644 --- a/README.md +++ b/README.md @@ -18,17 +18,20 @@ It is important to start doc-comments with the additional asterisk (`*`) -> `/** The content of the doc-comment should conform to the [Commonmark](https://spec.commonmark.org/0.30/) specification. -### Example +### Examples + +#### Example: simple but complete example The following is an example of markdown documentation for new and current users of nixdoc. -> Sidenote: Indentation is automatically detected and should be consistent across the content. -> +> Sidenote: Indentation is automatically detected and should be consistent across the content. +> > If you are used to multiline-strings (`''`) in nix this should be intuitive to follow. +`simple.nix` ````nix { - /** + /** This function adds two numbers # Example @@ -60,10 +63,39 @@ The following is an example of markdown documentation for new and current users > Note: Within nixpkgs the convention of using [definition-lists](https://www.markdownguide.org/extended-syntax/#definition-lists) for documenting arguments has been established. +## Example: using frontmatter + +In some cases, it may be desirable to store the documentation in a separate Markdown file. +Nixdoc supports frontmatter, which allows a separate Markdown file to be referenced within the doc comment. + +`fontmatter.nix` +````nix +{ + /** + --- + doc_location: ./docs.md + --- + */ + add = a: b: a + b; +} +```` + +`./docs.md` +````markdown +# Add + +A simple function that adds two numbers. +```` + +> Note: Frontmatter works only with the newer doc-comments `/** */`. + +See our [frontmatter specification](./doc/frontmatter.md) for further information. + +## Legacy support -## Custom nixdoc format (Legacy) +### Custom nixdoc format -You should consider migrating to the newer format described above. +You should consider migrating to the newer format described above. The format described in this section will be removed with the next major release. See [Migration guide](./doc/migration.md). diff --git a/doc/frontmatter.md b/doc/frontmatter.md index 510cd13..7e7f3f7 100644 --- a/doc/frontmatter.md +++ b/doc/frontmatter.md @@ -78,15 +78,41 @@ some nice docs! In this example, the `doc_location` directive fetches content from `./path.md` treating it as if it were directly within the doc-comment. This allows tracking the reference between the source position and the markdown file, in case of external documentation. -## Extensibility +#### Design decision: Relative vs Absolute paths -The initial set of keywords is intentionally minimalistic, focusing on immediate and broadly applicable needs. +This section justifies the previous decision why absolute paths are not allowed. + +
+Should absolute paths be allowed? + +- (+) When the docs are entirely elsewhere, e.g. `doc/manual/..`, a relative path would have to be `../../..`, very ugly + - (-) If only relative paths are allowed, encourages moving docs closer to the source, + Makes changing documentation easier. + - For the nix-build, adjustments which files are included in the derivation source may be needed. + - (-) With only relative paths, it's more similar to NixOS module docs +- (-) Makes it very confusing with where absolute paths are relative to (build root, git root, `.nix` location, etc.) +- (-) We can still allow absolute paths later on if necessary +- (-) Relies on a Git repository and git installed +- (-) It's unclear where absolute paths are rooted + - (+) Could use a syntax like `$GIT_ROOt/foo/bar` + - (-) Not a fan of more custom syntax + +**Decision**: Not supported by now. + +This outcome was discussed in the nix documentation team: https://discourse.nixos.org/t/2024-03-21-documentation-team-meeting-notes-114/41957. -Community contributions are encouraged to expand this list as new use cases emerge. +
## Error handling -Any issues encountered during the processing of frontmatter—be it syntax errors, invalid paths, or unsupported keywords—should result in clear, actionable error messages to the user. +Any issues encountered during the processing of frontmatter — be it syntax errors, invalid paths, or unsupported keywords—should result in clear, actionable error messages to the user. + +## Extensibility + +The initial set of keywords is intentionally minimalistic, focusing on immediate and broadly applicable needs. + +When extending this document we ask our contributors to be tooling agnostic, such that documentation wont't rely on any implementation details. +This approach ensures that the documentation remains independent of specific implementation details. By adhering to this principle, we aim to create a resource that is universally applicable ## Future work From 8f8680ac800fd51bf6313b7476a46d3e7ad54a52 Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Mon, 25 Mar 2024 10:51:35 +0100 Subject: [PATCH 8/9] Apply suggestions from code review Thanks @danielSidhion Co-authored-by: Daniel Sidhion --- README.md | 2 +- doc/frontmatter.md | 30 ++++++++++++------------------ 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 393c622..4d6b109 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,7 @@ The following is an example of markdown documentation for new and current users > Note: Within nixpkgs the convention of using [definition-lists](https://www.markdownguide.org/extended-syntax/#definition-lists) for documenting arguments has been established. -## Example: using frontmatter +#### Example: using frontmatter In some cases, it may be desirable to store the documentation in a separate Markdown file. Nixdoc supports frontmatter, which allows a separate Markdown file to be referenced within the doc comment. diff --git a/doc/frontmatter.md b/doc/frontmatter.md index 7e7f3f7..8af9385 100644 --- a/doc/frontmatter.md +++ b/doc/frontmatter.md @@ -34,17 +34,13 @@ Example: { /** --- - tags: - - foo - - bar + key: value --- */ foo = x: x; } ``` -In this example `key` is `tags` and `value` is `List("foo", "bar")`. - See also: [yaml specification](https://yaml.org/spec/1.2.2/) for how to use YAML. ## Keywords @@ -53,11 +49,11 @@ See also: [yaml specification](https://yaml.org/spec/1.2.2/) for how to use YAML Rules: -1. The `doc_location` keyword can be used to use content from another file INSTEAD of the doc-comments content. +1. The `doc_location` keyword can be used to use content from another file and **forbid** any further content to be added to the existing doc-comment. 2. The value must be given as a path relative to the current file. -3. Using this directive does not process any content inside the file, using it as-is. +3. The file pointed to by `doc_location` will be used as-is, without any further processing done to it. ```nix { @@ -75,31 +71,29 @@ Rules: some nice docs! ``` -In this example, the `doc_location` directive fetches content from `./path.md` treating it as if it were directly within the doc-comment. +In this example, the `doc_location` directive fetches content from `./path.md` and treats it as the actual doc-comment. This allows tracking the reference between the source position and the markdown file, in case of external documentation. #### Design decision: Relative vs Absolute paths -This section justifies the previous decision why absolute paths are not allowed. +This section explains the decision to only use relative paths and not support absolute paths for the file pointed to by `doc_location`.
Should absolute paths be allowed? - (+) When the docs are entirely elsewhere, e.g. `doc/manual/..`, a relative path would have to be `../../..`, very ugly - - (-) If only relative paths are allowed, encourages moving docs closer to the source, - Makes changing documentation easier. - - For the nix-build, adjustments which files are included in the derivation source may be needed. + - (-) If only relative paths are allowed, encourages moving docs closer to the source, makes changing documentation easier. + - For the nix-build, adjustments of which files are included in the derivation source may be needed. - (-) With only relative paths, it's more similar to NixOS module docs -- (-) Makes it very confusing with where absolute paths are relative to (build root, git root, `.nix` location, etc.) - (-) We can still allow absolute paths later on if necessary -- (-) Relies on a Git repository and git installed -- (-) It's unclear where absolute paths are rooted - - (+) Could use a syntax like `$GIT_ROOt/foo/bar` +- (-) Makes it very confusing where absolute paths are relative to (build root, git root, `.nix` location, etc.) + - (+) Could use a syntax like `$GIT_ROOT/foo/bar` + - (-) Relies on a Git repository and git installed - (-) Not a fan of more custom syntax **Decision**: Not supported by now. -This outcome was discussed in the nix documentation team: https://discourse.nixos.org/t/2024-03-21-documentation-team-meeting-notes-114/41957. +This outcome was discussed in the nix documentation team meeting: https://discourse.nixos.org/t/2024-03-21-documentation-team-meeting-notes-114/41957.
@@ -112,7 +106,7 @@ Any issues encountered during the processing of frontmatter — be it syntax err The initial set of keywords is intentionally minimalistic, focusing on immediate and broadly applicable needs. When extending this document we ask our contributors to be tooling agnostic, such that documentation wont't rely on any implementation details. -This approach ensures that the documentation remains independent of specific implementation details. By adhering to this principle, we aim to create a resource that is universally applicable +This approach ensures that the documentation remains independent of specific implementation details. By adhering to this principle, we aim to create a resource that is universally applicable. ## Future work From a429a29f07ff8ed6c8b0ecf3f35ed62ae554595a Mon Sep 17 00:00:00 2001 From: Johannes Kirschbauer Date: Mon, 25 Mar 2024 13:15:28 +0100 Subject: [PATCH 9/9] error handling: fail on conflicting doc_location and content --- src/frontmatter.rs | 12 ++++++++++++ src/test.rs | 21 ++++++++++++++++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/frontmatter.rs b/src/frontmatter.rs index c9ce91b..41f2056 100644 --- a/src/frontmatter.rs +++ b/src/frontmatter.rs @@ -19,6 +19,7 @@ pub struct Frontmatter { pub enum FrontmatterErrorKind { InvalidYaml, DocLocationFileNotFound, + DocLocationConflictWithContent, DocLocationNotRelativePath, } @@ -57,10 +58,21 @@ pub fn get_imported_content( } let pod = result.data.unwrap(); + let has_content = !result.content.trim().is_empty(); match pod.deserialize::() { Ok(metadata) => { let abs_import = match metadata.doc_location { Some(doc_location) => { + if has_content { + return Err(FrontmatterError { + message: format!( + "{:?}: doc_location: if this field is specified no other content is allowed for the doc-comment.", + file_path + ), + kind: FrontmatterErrorKind::DocLocationConflictWithContent, + }); + } + let import_path: PathBuf = PathBuf::from(&doc_location); let relative_path = RelativePath::from_path(&import_path); diff --git a/src/test.rs b/src/test.rs index fd89475..fe55883 100644 --- a/src/test.rs +++ b/src/test.rs @@ -241,7 +241,6 @@ fn test_frontmatter_doc_location_e2e() { const NOT_RELATIVE: &str = r#"--- doc_location: /tmp/not-relative.md --- -Other stuff "#; #[test] @@ -256,10 +255,27 @@ fn test_frontmatter_doc_location_relative() { ); } +const CONFLICT_DOCS: &str = r#"--- +doc_location: docs.md +--- +Other stuff +"#; + +#[test] +fn test_frontmatter_conflict_docs() { + let base_file = PathBuf::from("test/frontmatter-doc-location.nix"); + + let result = get_imported_content(&base_file, CONFLICT_DOCS); + + assert_eq!( + result.unwrap_err().kind, + FrontmatterErrorKind::DocLocationConflictWithContent + ); +} + const INVALID_TYPE: &str = r#"--- doc_location: 1 --- -Other stuff "#; #[test] @@ -274,7 +290,6 @@ fn test_frontmatter_doc_location_type() { const FILE_NOT_FOUND: &str = r#"--- doc_location: ./does-not-exist.md --- -Other stuff "#; #[test]