From 5fc841befbfc828198974143d376ff7bd4f57f66 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Wed, 25 Mar 2026 10:48:22 +0300 Subject: [PATCH 01/15] ndg-templates: remove debug logs from search script Not harmful to be really honest, but eh. Signed-off-by: NotAShelf Change-Id: If5c53c448174e96950085db355c6a1d06a6a6964 --- crates/ndg-templates/templates/search.js | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/crates/ndg-templates/templates/search.js b/crates/ndg-templates/templates/search.js index 9745ffdb..b168c1e5 100644 --- a/crates/ndg-templates/templates/search.js +++ b/crates/ndg-templates/templates/search.js @@ -62,7 +62,6 @@ class SearchEngine { throw new Error("Search data file not found at any expected location"); } - console.log(`Loading search data from: ${usedPath}`); if (!response.ok) { throw new Error(`HTTP ${response.status}: ${response.statusText}`); } @@ -87,7 +86,6 @@ class SearchEngine { this.initializeFromDocuments(documents); } this.isLoaded = true; - console.log(`Loaded ${documents.length} documents for search`); } catch (error) { console.error("Error loading search data:", error); this.documents = []; @@ -103,7 +101,6 @@ class SearchEngine { this.documents = []; } else { this.documents = documents; - console.log(`Initialized with ${documents.length} documents`); } try { await this.buildTokenMap(); @@ -177,9 +174,6 @@ class SearchEngine { if (endIndex < totalDocs) { setTimeout(() => processChunk(endIndex, chunkSize), 0); } else { - console.log( - `Built token map with ${this.tokenMap.size} unique tokens from ${processedDocs} documents`, - ); resolve(); } } catch (error) { @@ -383,7 +377,6 @@ class SearchEngine { } if (!this.isLoaded || this.documents.length === 0) { - console.log("Search data not available"); return []; } @@ -1043,7 +1036,6 @@ function initializeSearchWorker() { ? `${rootPath}assets/search-worker.js` : "/assets/search-worker.js"; searchWorker = new Worker(workerPath); - console.log("Web Worker initialized for background search"); return searchWorker; } catch (error) { console.warn("Web Worker creation failed, using main thread:", error); @@ -1065,9 +1057,7 @@ document.addEventListener("DOMContentLoaded", function () { // Initialize search engine immediately window.searchNamespace.engine .loadData() - .then(() => { - console.log("Search data loaded successfully"); - }) + .then(() => {}) .catch((error) => { console.error("Failed to initialize search:", error); }); From bda4a048143f639b8ccf4f57117cb0b00d3cbd21 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Wed, 25 Mar 2026 19:56:40 +0300 Subject: [PATCH 02/15] ndg-commonmark: extract shared DOM utilities; improve processor internals Signed-off-by: NotAShelf Change-Id: I686db149fc1cb48e0ac72dd3d04a37e06a6a6964 --- ndg-commonmark/src/processor/core.rs | 20 ++----------- ndg-commonmark/src/processor/dom.rs | 13 ++++++++ ndg-commonmark/src/processor/extensions.rs | 35 +++++++--------------- ndg-commonmark/src/processor/mod.rs | 4 ++- ndg-commonmark/src/processor/types.rs | 6 ---- ndg-commonmark/src/utils/mod.rs | 30 ++++++++++--------- 6 files changed, 46 insertions(+), 62 deletions(-) create mode 100644 ndg-commonmark/src/processor/dom.rs diff --git a/ndg-commonmark/src/processor/core.rs b/ndg-commonmark/src/processor/core.rs index e565a51c..05b74de4 100644 --- a/ndg-commonmark/src/processor/core.rs +++ b/ndg-commonmark/src/processor/core.rs @@ -1,8 +1,7 @@ //! Core implementation of the Markdown processor. //! -//! This module contains the main implementation of `MarkdownProcessor` and its -//! methods, focused on the core rendering pipeline and configuration -//! management. +//! Main implementation of `MarkdownProcessor` and its methods focused on the +//! core rendering pipeline and configuration management. use std::{ collections::HashMap, path::{Path, PathBuf}, @@ -30,21 +29,8 @@ pub enum DomError { /// Result type for DOM operations. pub type DomResult = Result; -/// Safely select DOM elements with graceful error handling. -fn safe_select( - document: &kuchikikiki::NodeRef, - selector: &str, -) -> Vec { - match document.select(selector) { - Ok(selections) => selections.map(|sel| sel.as_node().clone()).collect(), - Err(e) => { - log::warn!("DOM selector '{selector}' failed: {e:?}"); - Vec::new() - }, - } -} - use super::{ + dom::safe_select, process::process_safe, types::{ AstTransformer, diff --git a/ndg-commonmark/src/processor/dom.rs b/ndg-commonmark/src/processor/dom.rs new file mode 100644 index 00000000..a647f92f --- /dev/null +++ b/ndg-commonmark/src/processor/dom.rs @@ -0,0 +1,13 @@ +/// Safely select DOM elements with graceful error handling. +pub(super) fn safe_select( + document: &kuchikikiki::NodeRef, + selector: &str, +) -> Vec { + match document.select(selector) { + Ok(selections) => selections.map(|sel| sel.as_node().clone()).collect(), + Err(e) => { + log::warn!("DOM selector '{selector}' failed: {e:?}"); + Vec::new() + }, + } +} diff --git a/ndg-commonmark/src/processor/extensions.rs b/ndg-commonmark/src/processor/extensions.rs index 451c4ed6..796b6423 100644 --- a/ndg-commonmark/src/processor/extensions.rs +++ b/ndg-commonmark/src/processor/extensions.rs @@ -3,21 +3,7 @@ use std::{fmt::Write, fs, path::Path}; use html_escape::encode_text; -use super::process::process_safe; - -/// Safely select DOM elements with graceful error handling. -fn safe_select( - document: &kuchikikiki::NodeRef, - selector: &str, -) -> Vec { - match document.select(selector) { - Ok(selections) => selections.map(|sel| sel.as_node().clone()).collect(), - Err(e) => { - log::warn!("DOM selector '{selector}' failed: {e:?}"); - Vec::new() - }, - } -} +use super::{dom::safe_select, process::process_safe}; /// Apply GitHub Flavored Markdown (GFM) extensions to the input markdown. /// @@ -605,17 +591,18 @@ fn find_list_item_anchor(trimmed: &str) -> Option { } } - // Check for ordered list: "1. []{#id}" or "123. []{#id}" - let mut i = 0; - while i < trimmed.len() - && trimmed.chars().nth(i).unwrap_or(' ').is_ascii_digit() + // Check for ordered list: "1. []{#id}" or "123. []{#id}". + let digit_end = trimmed + .char_indices() + .find(|(_, c)| !c.is_ascii_digit()) + .map_or(trimmed.len(), |(i, _)| i); + if digit_end > 0 + && digit_end < trimmed.len() - 1 + && trimmed.as_bytes().get(digit_end) == Some(&b'.') { - i += 1; - } - if i > 0 && i < trimmed.len() - 1 && trimmed.chars().nth(i) == Some('.') { - let after_marker = &trimmed[i + 1..]; + let after_marker = &trimmed[digit_end + 1..]; if after_marker.starts_with(" []{#") { - return Some(i + 2); + return Some(digit_end + 2); } } diff --git a/ndg-commonmark/src/processor/mod.rs b/ndg-commonmark/src/processor/mod.rs index 9da0369f..eef058a1 100644 --- a/ndg-commonmark/src/processor/mod.rs +++ b/ndg-commonmark/src/processor/mod.rs @@ -9,11 +9,13 @@ //! The processor module is organized into focused submodules: //! //! - [`core`]: Main processor implementation and processing pipeline -//! - [`process`]: High-level processing functions with error recovery +//! - [`dom`]: DOM extraction helpers +//! - [`process`]: High-level processing functions //! - [`extensions`]: Feature-gated processing functions for different Markdown //! flavors //! - [`types`]: Core type definitions and configuration structures pub mod core; +pub mod dom; pub mod extensions; pub mod process; pub mod types; diff --git a/ndg-commonmark/src/processor/types.rs b/ndg-commonmark/src/processor/types.rs index d8887c47..2350d300 100644 --- a/ndg-commonmark/src/processor/types.rs +++ b/ndg-commonmark/src/processor/types.rs @@ -295,12 +295,6 @@ impl MarkdownOptionsBuilder { pub fn build(self) -> MarkdownOptions { self.options } - - /// Create options from external configuration with fluent interface. - #[must_use] - pub fn from_external_config(_config: &T) -> Self { - Self::new() - } } impl Default for MarkdownOptionsBuilder { diff --git a/ndg-commonmark/src/utils/mod.rs b/ndg-commonmark/src/utils/mod.rs index 187dac80..ea59f159 100644 --- a/ndg-commonmark/src/utils/mod.rs +++ b/ndg-commonmark/src/utils/mod.rs @@ -1,6 +1,6 @@ use std::{ collections::HashMap, - sync::{Mutex, OnceLock}, + sync::{LazyLock, OnceLock, RwLock}, }; pub mod codeblock; @@ -22,20 +22,20 @@ pub enum UtilError { /// Result type for utility operations. pub type UtilResult = Result; -/// Slugify a string for use as an anchor ID. -/// Converts to lowercase, replaces non-alphanumeric characters with dashes, -/// and trims leading/trailing dashes. +/// Slugify a string for use as an anchor ID. Converts to lowercase, replaces +/// non-alphanumeric characters with dashes, and trims leading/trailing dashes. #[must_use] pub fn slugify(text: &str) -> String { - static CACHE: Mutex>> = Mutex::new(None); - let mut cache = CACHE - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); + static CACHE: LazyLock>> = + LazyLock::new(|| RwLock::new(HashMap::new())); - if let Some(ref map) = *cache - && let Some(cached) = map.get(text) { - return cached.clone(); + let cache = CACHE + .read() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if let Some(cached) = cache.get(text) { + return cached.clone(); + } } let result = text @@ -44,9 +44,11 @@ pub fn slugify(text: &str) -> String { .trim_matches('-') .to_string(); - let map = cache.get_or_insert_with(HashMap::new); - if map.len() < 2048 { - map.insert(text.to_string(), result.clone()); + let mut cache = CACHE + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner); + if cache.len() < 2048 { + cache.insert(text.to_string(), result.clone()); } result From a91539277734b91bae4387efaeadef94e1ffded7 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Wed, 25 Mar 2026 20:22:28 +0300 Subject: [PATCH 03/15] ndg-macros: replace string-based type matching with AST helpers Introduces `type_is`, `last_type_segment`, and `first_type_arg` helpers that inspect the `syn` type tree directly and removes the fragile `to_token_stream()` string comparisons that I always hated. Signed-off-by: NotAShelf Change-Id: I7bfadff11b808ce4b46decf07ad7e0ea6a6a6964 --- crates/ndg-macros/src/lib.rs | 265 ++++++++++++++++++++--------------- 1 file changed, 154 insertions(+), 111 deletions(-) diff --git a/crates/ndg-macros/src/lib.rs b/crates/ndg-macros/src/lib.rs index 10c186f1..b2c290e2 100644 --- a/crates/ndg-macros/src/lib.rs +++ b/crates/ndg-macros/src/lib.rs @@ -1,9 +1,8 @@ //! Proc-macros for NDG configuration system. //! //! Provides derive macros for automatic configuration handling. - use proc_macro::TokenStream; -use quote::{ToTokens, quote}; +use quote::quote; use syn::{Attribute, Data, DeriveInput, Fields, Type, parse_macro_input}; /// Attribute configuration for a field. @@ -61,6 +60,36 @@ impl FieldConfig { } } +/// Get the last path segment of a type, if it is a simple path type. +fn last_type_segment(ty: &syn::Type) -> Option<&syn::Ident> { + if let syn::Type::Path(tp) = ty { + tp.path.segments.last().map(|s| &s.ident) + } else { + None + } +} + +/// Returns true if the type's last path segment matches `name`. Works for +/// `Option`, `Vec`, `HashMap`, etc. +fn type_is(ty: &syn::Type, name: &str) -> bool { + last_type_segment(ty).is_some_and(|ident| ident == name) +} + +/// Get the first generic type argument of a type (e.g., T from `Option`). +fn first_type_arg(ty: &syn::Type) -> Option<&syn::Type> { + if let syn::Type::Path(tp) = ty + && let Some(last) = tp.path.segments.last() + && let syn::PathArguments::AngleBracketed(args) = &last.arguments + { + for arg in &args.args { + if let syn::GenericArgument::Type(inner) = arg { + return Some(inner); + } + } + } + None +} + /// Derive macro for configuration structs. #[proc_macro_derive(Configurable, attributes(config))] pub fn derive_configurable(input: TokenStream) -> TokenStream { @@ -229,28 +258,29 @@ fn generate_field_handler( } // Skip Vec and HashMap fields, they can't be directly overridden - let type_str = field_type.to_token_stream().to_string(); - if type_str.starts_with("Vec<") || type_str.contains("HashMap<") { + if type_is(field_type, "Vec") || type_is(field_type, "HashMap") { return quote! {}; } // Handle deprecation - let deprecation_check = - if let Some((version, replacement)) = &config.deprecated { - let msg = if let Some(replacement) = replacement { + let deprecation_check = if let Some((version, replacement)) = + &config.deprecated + { + let msg = replacement.as_ref().map_or_else( + || format!("The '{field_key}' config key is deprecated since {version}."), + |replacement| { format!( "The '{field_key}' config key is deprecated since {version}. Use \ '{replacement}' instead." ) - } else { - format!("The '{field_key}' config key is deprecated since {version}.") - }; - quote! { - log::warn!(#msg); - } - } else { - quote! {} - }; + }, + ); + quote! { + log::warn!(#msg); + } + } else { + quote! {} + }; // Generate type-specific parsing let value_assignment = @@ -301,11 +331,74 @@ fn generate_value_assignment( field_type: &Type, config: &FieldConfig, ) -> proc_macro2::TokenStream { - let type_str = field_type.to_token_stream().to_string(); + // Option handling, must come before general Option check + if type_is(field_type, "Option") + && first_type_arg(field_type) + .is_some_and(|inner| type_is(inner, "SidebarOrdering")) + { + return quote! { + self.#field_name = if value.is_empty() { + None + } else { + Some(value.parse().map_err(|e: String| ConfigError::Config(format!( + "Invalid value for '{}': '{}' - {}", + stringify!(#field_name), value, e + )))?) + }; + }; + } - // Option handling - if type_str.starts_with("Option ") || type_str.starts_with("Option<") { - if config.allow_empty { + // Option handling, must come before general Option check + if type_is(field_type, "Option") + && first_type_arg(field_type).is_some_and(|inner| type_is(inner, "usize")) + { + return quote! { + self.#field_name = if value.is_empty() { + None + } else { + Some(value.parse().map_err(|_| ConfigError::Config(format!( + "Invalid value for '{}': '{}'. Expected a positive integer", + stringify!(#field_name), value + )))?) + }; + }; + } + + // Option handling, must come before general Option check + if type_is(field_type, "Option") + && first_type_arg(field_type).is_some_and(|inner| type_is(inner, "u8")) + { + return quote! { + self.#field_name = if value.is_empty() { + None + } else { + Some(value.parse().map_err(|_| ConfigError::Config(format!( + "Invalid value for '{}': '{}'. Expected a number between 0-255", + stringify!(#field_name), value + )))?) + }; + }; + } + + // Option handling, must come before general Option check + if type_is(field_type, "Option") + && first_type_arg(field_type).is_some_and(|inner| type_is(inner, "f32")) + { + return quote! { + self.#field_name = if value.is_empty() { + None + } else { + Some(value.parse().map_err(|_| ConfigError::Config(format!( + "Invalid value for '{}': '{}'. Expected a number", + stringify!(#field_name), value + )))?) + }; + }; + } + + // General Option handling + if type_is(field_type, "Option") { + return if config.allow_empty { quote! { self.#field_name = if value.is_empty() { None @@ -321,23 +414,26 @@ fn generate_value_assignment( format!("Invalid value for '{}': '{}'", stringify!(#field_name), value) ))?); } - } + }; } + // PathBuf handling - else if type_str.contains("PathBuf") { - quote! { + if type_is(field_type, "PathBuf") { + return quote! { self.#field_name = std::path::PathBuf::from(value); - } + }; } + // String handling - else if type_str == "String" { - quote! { + if type_is(field_type, "String") { + return quote! { self.#field_name = value.to_string(); - } + }; } + // Bool handling - else if type_str == "bool" { - quote! { + if type_is(field_type, "bool") { + return quote! { self.#field_name = match value.to_lowercase().as_str() { "true" | "yes" | "1" => true, "false" | "no" | "0" => false, @@ -348,106 +444,55 @@ fn generate_value_assignment( ))); } }; - } + }; } + // SidebarOrdering handling - else if type_str == "SidebarOrdering" { - quote! { + if type_is(field_type, "SidebarOrdering") { + return quote! { self.#field_name = value.parse().map_err(|e: String| ConfigError::Config(format!( "Invalid value for '{}': '{}' - {}", stringify!(#field_name), value, e )))?; - } - } - // Option handling - else if type_str.contains("Option") - || type_str.contains("Option < SidebarOrdering >") - { - quote! { - self.#field_name = if value.is_empty() { - None - } else { - Some(value.parse().map_err(|e: String| ConfigError::Config(format!( - "Invalid value for '{}': '{}' - {}", - stringify!(#field_name), value, e - )))?) - }; - } + }; } + // usize handling - else if type_str == "usize" { - quote! { + if type_is(field_type, "usize") { + return quote! { self.#field_name = value.parse().map_err(|_| ConfigError::Config(format!( "Invalid value for '{}': '{}'. Expected a positive integer", stringify!(#field_name), value )))?; - } - } - // Option handling - else if type_str.contains("Option") { - quote! { - self.#field_name = if value.is_empty() { - None - } else { - Some(value.parse().map_err(|_| ConfigError::Config(format!( - "Invalid value for '{}': '{}'. Expected a positive integer", - stringify!(#field_name), value - )))?) - }; - } + }; } + // u8 handling - else if type_str == "u8" { - quote! { + if type_is(field_type, "u8") { + return quote! { self.#field_name = value.parse().map_err(|_| ConfigError::Config(format!( "Invalid value for '{}': '{}'. Expected a number between 0-255", stringify!(#field_name), value )))?; - } - } - // Option handling - else if type_str.contains("Option") { - quote! { - self.#field_name = if value.is_empty() { - None - } else { - Some(value.parse().map_err(|_| ConfigError::Config(format!( - "Invalid value for '{}': '{}'. Expected a number between 0-255", - stringify!(#field_name), value - )))?) - }; - } + }; } + // f32 handling - else if type_str == "f32" { - quote! { + if type_is(field_type, "f32") { + return quote! { self.#field_name = value.parse().map_err(|_| ConfigError::Config(format!( "Invalid value for '{}': '{}'. Expected a number", stringify!(#field_name), value )))?; - } - } - // Option handling - else if type_str.contains("Option") { - quote! { - self.#field_name = if value.is_empty() { - None - } else { - Some(value.parse().map_err(|_| ConfigError::Config(format!( - "Invalid value for '{}': '{}'. Expected a number", - stringify!(#field_name), value - )))?) - }; - } + }; } + // Default: try to parse - else { - quote! { - self.#field_name = value.parse().map_err(|_| ConfigError::Config(format!( - "Invalid value for '{}': '{}'", - stringify!(#field_name), value - )))?; - } + quote! { + self.#field_name = value.parse().map_err(|_| ConfigError::Config(format!( + "Invalid value for '{}': '{}'", + stringify!(#field_name), value + )))?; } } @@ -458,7 +503,6 @@ fn generate_merge_handlers(fields: &Fields) -> Vec { let field_config = FieldConfig::from_attrs(&field.attrs); let field_name = field.ident.as_ref().expect("Named field required"); let field_type = &field.ty; - let type_str = field_type.to_token_stream().to_string(); let handler = if field_config.nested { // For nested configs, replace if other has Some @@ -467,8 +511,9 @@ fn generate_merge_handlers(fields: &Fields) -> Vec { self.#field_name = other.#field_name; } } - } else if type_str.starts_with("Option fields: extend if both Some, otherwise replace only if // other is Some @@ -483,21 +528,19 @@ fn generate_merge_handlers(fields: &Fields) -> Vec { _ => {} } } - } else if type_str.starts_with("Option<") - || type_str.starts_with("Option <") - { + } else if type_is(field_type, "Option") { // Option fields: replace if other has Some quote! { if other.#field_name.is_some() { self.#field_name = other.#field_name; } } - } else if type_str.starts_with("Vec<") || type_str.starts_with("Vec <") { + } else if type_is(field_type, "Vec") { // Vec fields: extend quote! { self.#field_name.extend(other.#field_name); } - } else if type_str.contains("HashMap") { + } else if type_is(field_type, "HashMap") { // HashMap fields: extend (other takes precedence) quote! { self.#field_name.extend(other.#field_name); From 3aee6c51868e574d4876dafb7d166db84083cfc4 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Wed, 25 Mar 2026 20:25:25 +0300 Subject: [PATCH 04/15] ndg-utils/ndg-html: consolidate markdown utilities Moves `extract_page_title` helper to `ndg-utils::markdown`, and removes the local copy from `ndg-html`. `collect_included_files` is now superseded by `process_markdown_files` populating `config.included_files` as a side-effect, so it's removed entirely. Signed-off-by: NotAShelf Change-Id: I66eb51184c4dd264e5b74cb3d8063b306a6a6964 --- crates/ndg-html/src/template.rs | 43 +---------- .../ndg-html/tests/search_path_resolution.rs | 8 +- crates/ndg-utils/src/lib.rs | 6 +- crates/ndg-utils/src/markdown.rs | 76 ------------------- 4 files changed, 8 insertions(+), 125 deletions(-) diff --git a/crates/ndg-html/src/template.rs b/crates/ndg-html/src/template.rs index 3e440227..e6c72afe 100644 --- a/crates/ndg-html/src/template.rs +++ b/crates/ndg-html/src/template.rs @@ -1006,45 +1006,6 @@ struct NavItem { number: Option, } -/// Extract page title from markdown file. -/// -/// First attempts to read the file and extract the title from the first H1 -/// heading. Falls back to using the file stem as the title if reading fails or -/// no H1 is found. -fn extract_page_title(path: &Path, html_path: &Path) -> String { - std::fs::read_to_string(path).map_or_else( - |_| { - html_path - .file_stem() - .unwrap_or_default() - .to_string_lossy() - .to_string() - }, - |content| { - content.lines().next().map_or_else( - || { - html_path - .file_stem() - .unwrap_or_default() - .to_string_lossy() - .to_string() - }, - |first_line| { - first_line.strip_prefix("# ").map_or_else( - || { - html_path - .file_stem() - .unwrap_or_default() - .to_string_lossy() - .to_string() - }, - ndg_commonmark::utils::clean_anchor_patterns, - ) - }, - ) - }, - ) -} /// Generate the document navigation HTML fn generate_doc_nav(config: &Config, current_file_rel_path: &Path) -> String { @@ -1105,7 +1066,7 @@ fn generate_doc_nav(config: &Config, current_file_rel_path: &Path) -> String { format!("{}{}", root_prefix, html_path.to_string_lossy()); // Extract page title - let page_title = extract_page_title(path, &html_path); + let page_title = ndg_utils::markdown::extract_page_title(path, &html_path); // Apply sidebar configuration if available let (display_title, position) = @@ -1181,7 +1142,7 @@ fn generate_doc_nav(config: &Config, current_file_rel_path: &Path) -> String { let target_path = format!("{}{}", root_prefix, html_path.to_string_lossy()); - let page_title = extract_page_title(path, &html_path); + let page_title = ndg_utils::markdown::extract_page_title(path, &html_path); // Apply sidebar configuration to special files if available let display_title = if let Some(sidebar_config) = &config.sidebar { diff --git a/crates/ndg-html/tests/search_path_resolution.rs b/crates/ndg-html/tests/search_path_resolution.rs index ecd47a90..a846fdfb 100644 --- a/crates/ndg-html/tests/search_path_resolution.rs +++ b/crates/ndg-html/tests/search_path_resolution.rs @@ -15,7 +15,7 @@ use ndg_html::{ search::{SearchData, generate_search_index}, template::render, }; -use ndg_utils::{collect_included_files, markdown::create_processor}; +use ndg_utils::{markdown::create_processor, process_markdown_files}; use serde_json::json; use tempfile::TempDir; @@ -199,8 +199,10 @@ This file should be transitively included in `main.html` }; let processor = Some(create_processor(&config, None)); - config.included_files = collect_included_files(&config, processor.as_ref()) - .expect("Failed to collect include files"); + + // Populate config.included_files as a side effect of processing markdown files + let _processed_files = process_markdown_files(&mut config, processor.as_ref()) + .expect("Failed to process markdown files"); let all_markdown_files = collect_markdown_files(&input_dir); diff --git a/crates/ndg-utils/src/lib.rs b/crates/ndg-utils/src/lib.rs index ad233f00..aefabc1b 100644 --- a/crates/ndg-utils/src/lib.rs +++ b/crates/ndg-utils/src/lib.rs @@ -7,9 +7,5 @@ pub mod postprocess; // Re-export commonly used utilities pub use assets::copy_assets; -pub use markdown::{ - collect_included_files, - create_processor, - process_markdown_files, -}; +pub use markdown::{create_processor, process_markdown_files}; pub use output::create_fallback_index; diff --git a/crates/ndg-utils/src/markdown.rs b/crates/ndg-utils/src/markdown.rs index 4126f68d..afaf7287 100644 --- a/crates/ndg-utils/src/markdown.rs +++ b/crates/ndg-utils/src/markdown.rs @@ -108,82 +108,6 @@ pub struct ProcessedMarkdown { pub frontmatter: Option, } -/// Collects all included files from markdown documents in the input directory. -/// -/// This scans all markdown files and collects the set of files that are -/// included via NDG's markdown processor. -/// -/// # Arguments -/// -/// * `config` - The loaded configuration for documentation generation. -/// * `processor` - Optional pre-created processor to reuse. If None, creates a -/// new one. -/// -/// # Returns -/// -/// A mapping of included file paths (relative to input directory) to the parent -/// input file path, which included that file -/// -/// # Errors -/// -/// Returns an error if any markdown file cannot be read. -#[allow(dead_code, reason = "Kept for backward compatibility")] -pub fn collect_included_files( - config: &Config, - processor: Option<&MarkdownProcessor>, -) -> Result> { - let mut all_included_files: HashMap = HashMap::new(); - - if let Some(ref input_dir) = config.input_dir { - let files = collect_markdown_files(input_dir); - - // Use provided processor or create a new one - let base_processor = processor - .map_or_else(|| create_processor(config, None), std::clone::Clone::clone); - - for file_path in &files { - let content = fs::read_to_string(file_path).wrap_err_with(|| { - format!("Failed to read markdown file: {}", file_path.display()) - })?; - - let base_dir = file_path.parent().unwrap_or_else(|| { - config - .input_dir - .as_deref() - .unwrap_or_else(|| Path::new(".")) - }); - let processor = base_processor.clone().with_base_dir(base_dir); - - let (_, content) = strip_frontmatter(&content); - let result = processor.render(&content); - - for inc in &result.included_files { - // Include paths are relative to the file's directory, not input_dir - let inc_path = base_dir.join(&inc.path); - if let Ok(inc_rel) = inc_path.strip_prefix(input_dir) { - let Ok(includer_rel) = file_path.strip_prefix(input_dir) else { - continue; - }; - - all_included_files - .entry(inc_rel.to_path_buf()) - .and_modify(|old| { - // For deterministic output - if includer_rel < old.as_path() { - *old = { - let this = &includer_rel; - this.to_path_buf() - }; - } - }) - .or_insert_with(|| includer_rel.to_owned()); - } - } - } - } - - Ok(all_included_files) -} /// Processes all markdown files in the input directory and returns processed /// data. From 5bfd371f7d202161ed149f260358ce1da5d1c302 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Wed, 25 Mar 2026 21:08:42 +0300 Subject: [PATCH 05/15] ndg-commonmark: general unsafe/unsound code cleanup; fix correctness issues Signed-off-by: NotAShelf Change-Id: Ib532f0768cd6dd28866196fc809f5b316a6a6964 --- ndg-commonmark/src/processor/core.rs | 52 ++------------------- ndg-commonmark/src/processor/extensions.rs | 54 +++++++++++----------- ndg-commonmark/src/syntax/syntastica.rs | 29 ++++-------- ndg-commonmark/src/utils/mod.rs | 21 ++------- 4 files changed, 45 insertions(+), 111 deletions(-) diff --git a/ndg-commonmark/src/processor/core.rs b/ndg-commonmark/src/processor/core.rs index 05b74de4..851222cd 100644 --- a/ndg-commonmark/src/processor/core.rs +++ b/ndg-commonmark/src/processor/core.rs @@ -17,18 +17,6 @@ use log::trace; use markup5ever::local_name; use walkdir::WalkDir; -/// Error type for DOM operations. -#[derive(Debug, thiserror::Error)] -pub enum DomError { - #[error("CSS selector failed: {0}")] - SelectorError(String), - #[error("DOM serialization failed: {0}")] - SerializationError(String), -} - -/// Result type for DOM operations. -pub type DomResult = Result; - use super::{ dom::safe_select, process::process_safe, @@ -206,6 +194,7 @@ impl MarkdownProcessor { /// Process hard tabs in code blocks within markdown content fn process_hardtabs(&self, markdown: &str) -> String { use super::types::TabStyle; + use crate::utils::codeblock::FenceTracker; // If no tab handling is needed, return as-is if self.options.tab_style == TabStyle::None { @@ -214,44 +203,13 @@ impl MarkdownProcessor { let mut result = String::with_capacity(markdown.len()); let mut lines = markdown.lines().peekable(); - let mut in_code_block = false; - let mut code_fence_char = None; - let mut code_fence_count = 0; + let mut tracker = FenceTracker::new(); while let Some(line) = lines.next() { - let trimmed = line.trim_start(); - - // Check for code fences - if trimmed.starts_with("```") || trimmed.starts_with("~~~") { - let Some(fence_char) = trimmed.chars().next() else { - // If the line is empty after trimming, it can't be a valid code fence - // Just continue processing the line normally - result.push_str(line); - result.push('\n'); - continue; - }; - let fence_count = - trimmed.chars().take_while(|&c| c == fence_char).count(); - - if fence_count >= 3 { - if !in_code_block { - // Starting a code block - in_code_block = true; - code_fence_char = Some(fence_char); - code_fence_count = fence_count; - } else if code_fence_char == Some(fence_char) - && fence_count >= code_fence_count - { - // Ending a code block - in_code_block = false; - code_fence_char = None; - code_fence_count = 0; - } - } - } + tracker = tracker.process_line(line); - // Process line based on whether we're in a code block - let processed_line = if in_code_block && line.contains('\t') { + // Only replace tabs inside fenced code blocks + let processed_line = if tracker.in_code_block() && line.contains('\t') { self.handle_hardtabs(line) } else { line.to_string() diff --git a/ndg-commonmark/src/processor/extensions.rs b/ndg-commonmark/src/processor/extensions.rs index 796b6423..06ebda78 100644 --- a/ndg-commonmark/src/processor/extensions.rs +++ b/ndg-commonmark/src/processor/extensions.rs @@ -535,11 +535,6 @@ fn process_line_myst_autolinks(line: &str) -> String { /// # Returns /// /// The processed markdown with inline anchors converted to HTML spans -/// -/// # Panics -/// -/// Panics if a code fence marker line is empty (which should not occur in valid -/// markdown). #[cfg(feature = "nixpkgs")] #[must_use] pub fn process_inline_anchors(content: &str) -> String { @@ -710,15 +705,12 @@ fn process_line_anchors(line: &str) -> String { /// code fences. /// /// # Arguments +/// /// * `content` - The markdown content to process /// /// # Returns -/// The processed markdown with block elements converted to HTML /// -/// # Panics -/// -/// Panics if a code fence marker line is empty (which should not occur in valid -/// markdown). +/// The processed markdown with block elements converted to HTML #[cfg(feature = "nixpkgs")] #[must_use] pub fn process_block_elements(content: &str) -> String { @@ -801,19 +793,22 @@ fn parse_github_callout(line: &str) -> Option<(String, String)> { /// /// Per `CommonMark` spec, an ATX header requires 1-6 '#' characters followed by /// either: +/// /// - A whitespace character (space, tab, etc.) /// - End of line (the string ends) /// /// # Arguments +/// /// * `line` - The line to check /// /// # Returns +/// /// `true` if the line starts with a valid ATX header marker fn is_atx_header(line: &str) -> bool { let mut chars = line.chars(); let mut hash_count = 0; - // count leading '#' characters (max 6) + // Count leading '#' characters (max 6) while let Some(c) = chars.next() { if c == '#' { hash_count += 1; @@ -821,13 +816,13 @@ fn is_atx_header(line: &str) -> bool { return false; } } else { - // found a non-'#' character, check if it's whitespace or we're at EOL + // Found a non-'#' character, check if it's whitespace or we're at EOL return (1..=6).contains(&hash_count) && (c.is_whitespace() || chars.as_str().is_empty()); } } - // reached end of string, check if we have 1-6 hashes + // Reached end of string, check if we have 1-6 hashes (1..=6).contains(&hash_count) } @@ -873,7 +868,7 @@ fn collect_github_callout_content( } // Lazy continuation - // Mind yu, "lazy" doesn't refer to me being lazy but the GFM feature for + // Mind you, "lazy" doesn't refer to me being lazy but the GFM feature for // a line without `>` that continues the blockquote // paragraph trimmed @@ -903,7 +898,7 @@ fn parse_fenced_admonition_start( // Find the closing brace if let Some(close_brace) = after_colons.find('}') { - let content = &after_colons[2..close_brace]; // Skip "{." + let content = &after_colons[2..close_brace]; // skip "{." // Parse type and optional ID let parts: Vec<&str> = content.split_whitespace().collect(); @@ -911,7 +906,7 @@ fn parse_fenced_admonition_start( let id = parts .iter() .find(|part| part.starts_with('#')) - .map(|id_part| id_part[1..].to_string()); // Remove '#' + .map(|id_part| id_part[1..].to_string()); // remove '#' return Some((adm_type.to_string(), id)); } @@ -922,9 +917,11 @@ fn parse_fenced_admonition_start( /// Collect content until closing ::: /// -/// Returns a tuple of (`admonition_content`, `trailing_content`). -/// If there's content after the closing `:::` on the same line, -/// it's returned as `trailing_content`. +/// # Returns +/// +/// Tuple of (`admonition_content`, `trailing_content`). If there's content +/// after the closing `:::` on the same line, it's returned as +/// `trailing_content`. fn collect_fenced_content( lines: &mut std::iter::Peekable, ) -> (String, Option) { @@ -1033,17 +1030,17 @@ fn render_figure(id: Option<&str>, title: &str, content: &str) -> String { ) } -/// Process manpage references in HTML content. -/// -/// This function processes manpage references by finding span elements with -/// manpage-reference class and converting them to links when URLs are -/// available. +/// Process manpage references in HTML content. Pocesses manpage references by +/// finding span elements with manpage-reference class and converting them to +/// links when URLs are available. /// /// # Arguments +/// /// * `html` - The HTML content to process /// * `manpage_urls` - Optional mapping of manpage names to URLs /// /// # Returns +/// /// The processed HTML with manpage references converted to links #[cfg(feature = "nixpkgs")] #[must_use] @@ -1110,7 +1107,7 @@ pub fn process_manpage_references( let mut out = Vec::new(); let _ = document.serialize(&mut out); - String::from_utf8(out).unwrap_or_default() + String::from_utf8(out).unwrap_or_else(|_| html.to_string()) }, // Return original HTML on error "", @@ -1175,9 +1172,10 @@ pub fn process_option_references( } if !is_already_option_ref { - // Check if validation is enabled and option is valid + // Check if validation is enabled and option is valid. If no + // validation set, link all options let should_link = - valid_options.is_none_or(|opts| opts.contains(code_text.as_str())); // If no validation set, link all options + valid_options.is_none_or(|opts| opts.contains(code_text.as_str())); if should_link { let option_id = format!("option-{}", code_text.replace('.', "-")); @@ -1214,7 +1212,7 @@ pub fn process_option_references( let mut out = Vec::new(); let _ = document.serialize(&mut out); - String::from_utf8(out).unwrap_or_default() + String::from_utf8(out).unwrap_or_else(|_| html.to_string()) }, // Return original HTML on error "", diff --git a/ndg-commonmark/src/syntax/syntastica.rs b/ndg-commonmark/src/syntax/syntastica.rs index 4e5a81b4..71ef8068 100644 --- a/ndg-commonmark/src/syntax/syntastica.rs +++ b/ndg-commonmark/src/syntax/syntastica.rs @@ -8,16 +8,14 @@ //! //! We programmatically load all available themes from `syntastica-themes` //! Some of the popular themes included are: +//! //! - github (dark/light variants) //! - gruvbox (dark/light) //! - nord, dracula, catppuccin //! - tokyo night, solarized, monokai //! - And many more... -use std::{ - collections::HashMap, - sync::{Arc, Mutex}, -}; +use std::{collections::HashMap, sync::Mutex}; use syntastica::{Processor, render, renderer::HtmlRenderer}; use syntastica_core::theme::ResolvedTheme; @@ -30,11 +28,6 @@ use super::{ /// Syntastica-based syntax highlighter. pub struct SyntasticaHighlighter { - #[allow( - dead_code, - reason = "Must be kept alive as processor holds reference to it" - )] - language_set: Arc, themes: HashMap, default_theme: ResolvedTheme, processor: Mutex>, @@ -49,8 +42,6 @@ impl SyntasticaHighlighter { /// Currently never returns an error, but returns a Result for API /// consistency. pub fn new() -> SyntaxResult { - let language_set = Arc::new(LanguageSetImpl::new()); - let mut themes = HashMap::new(); // Load all available themes @@ -62,16 +53,16 @@ impl SyntasticaHighlighter { let default_theme = syntastica_themes::one::dark(); - // Create processor with a static reference to the language set - // Safety: The Arc ensures the language set outlives the processor - let processor = unsafe { - let language_set_ref: &'static LanguageSetImpl = - &*std::ptr::from_ref::(language_set.as_ref()); - Processor::new(language_set_ref) - }; + // Leak the language set into a `'static` reference so the `Processor` can + // hold it for the remainder of the process lifetime. This is sound for a + // CLI: the process exits when documentation generation completes and the OS + // reclaims the memory. It avoids the unsound lifetime fabrication that a + // raw-pointer cast would require. + let language_set_static: &'static LanguageSetImpl = + Box::leak(Box::new(LanguageSetImpl::new())); + let processor = Processor::new(language_set_static); Ok(Self { - language_set, themes, default_theme, processor: Mutex::new(processor), diff --git a/ndg-commonmark/src/utils/mod.rs b/ndg-commonmark/src/utils/mod.rs index ea59f159..48d66e88 100644 --- a/ndg-commonmark/src/utils/mod.rs +++ b/ndg-commonmark/src/utils/mod.rs @@ -70,7 +70,10 @@ pub fn extract_markdown_title(content: &str) -> Option { let root = parse_document(&arena, content, &options); for node in root.descendants() { - if let NodeValue::Heading(_) = &node.data.borrow().value { + if let NodeValue::Heading(NodeHeading { level, .. }) = + &node.data.borrow().value + && *level == 1 + { let text = extract_inline_text_from_node(node); if !text.trim().is_empty() { return Some(text.trim().to_string()); @@ -205,22 +208,6 @@ pub fn clean_anchor_patterns(text: &str) -> String { anchor_pattern.replace_all(text.trim(), "").to_string() } -/// Apply a regex transformation to HTML elements using the provided function. -/// Used by the markdown processor for HTML element transformations. -pub fn process_html_elements( - html: &str, - regex: &Regex, - transform: F, -) -> String -where - F: Fn(®ex::Captures) -> String, -{ - match regex.replace_all(html, transform) { - std::borrow::Cow::Borrowed(_) => html.to_string(), - std::borrow::Cow::Owned(s) => s, - } -} - /// Strip markdown formatting and return plain text. /// /// This processes the markdown through the AST and extracts only text content, From 6900f6272e5a46b25514ea6a23dc599fd5dbc12b Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Wed, 25 Mar 2026 23:10:50 +0300 Subject: [PATCH 06/15] ndg-macros: fix two-pass deprecated field parsing; wrap generated items Signed-off-by: NotAShelf Change-Id: I6768c5531746b4fe89a04e8033ef115a6a6a6964 --- crates/ndg-macros/src/lib.rs | 160 ++++++++++++++++++----------------- 1 file changed, 84 insertions(+), 76 deletions(-) diff --git a/crates/ndg-macros/src/lib.rs b/crates/ndg-macros/src/lib.rs index b2c290e2..d4dfa44a 100644 --- a/crates/ndg-macros/src/lib.rs +++ b/crates/ndg-macros/src/lib.rs @@ -19,9 +19,6 @@ struct FieldConfig { /// Allow empty values (set to None). allow_empty: bool, - - /// Pending replacement value (set before deprecated). - pending_replacement: Option, } impl FieldConfig { @@ -33,6 +30,13 @@ impl FieldConfig { continue; } + // Collect deprecated_version and replacement independently so + // that `#[config(deprecated = "v", replacement = "k")]` and + // `#[config(replacement = "k", deprecated = "v")]` produce identical + // results regardless of attribute order. + let mut deprecated_version: Option = None; + let mut pending_replacement: Option = None; + let _ = attr.parse_nested_meta(|meta| { if meta.path.is_ident("key") { let value = meta.value()?; @@ -41,12 +45,11 @@ impl FieldConfig { } else if meta.path.is_ident("deprecated") { let value = meta.value()?; let lit: syn::LitStr = value.parse()?; - config.deprecated = - Some((lit.value(), config.pending_replacement.take())); + deprecated_version = Some(lit.value()); } else if meta.path.is_ident("replacement") { let value = meta.value()?; let lit: syn::LitStr = value.parse()?; - config.pending_replacement = Some(lit.value()); + pending_replacement = Some(lit.value()); } else if meta.path.is_ident("allow_empty") { config.allow_empty = true; } else if meta.path.is_ident("nested") { @@ -54,6 +57,10 @@ impl FieldConfig { } Ok(()) }); + + if let Some(version) = deprecated_version { + config.deprecated = Some((version, pending_replacement)); + } } config @@ -131,86 +138,87 @@ pub fn derive_configurable(input: TokenStream) -> TokenStream { ); let expanded = quote! { - impl #impl_generics #name #ty_generics #where_clause { - /// Apply a configuration override by key. - pub fn apply_override( - &mut self, - key: &str, - value: &str, - ) -> std::result::Result<(), crate::error::ConfigError> { - use crate::error::ConfigError; - - #(#field_handlers)* - - // Generate "did you mean" suggestions - let valid_keys: &[&str] = &[#(#valid_keys_ref),*]; - let suggestions = #find_similar_fn(key, valid_keys, 3, 0.5); - - let error_msg = if suggestions.is_empty() { - format!("Unknown configuration key: '{key}'. See documentation for supported keys.") - } else { - let suggestions_str = suggestions.join("', '"); - format!("Unknown configuration key: '{key}'. Did you mean: '{suggestions_str}'?") - }; + const _: () = { + impl #impl_generics #name #ty_generics #where_clause { + /// Apply a configuration override by key. + pub fn apply_override( + &mut self, + key: &str, + value: &str, + ) -> std::result::Result<(), crate::error::ConfigError> { + use crate::error::ConfigError; + + #(#field_handlers)* + + // Generate "did you mean" suggestions + let valid_keys: &[&str] = &[#(#valid_keys_ref),*]; + let suggestions = #find_similar_fn(key, valid_keys, 3, 0.5); + + let error_msg = if suggestions.is_empty() { + format!("Unknown configuration key: '{key}'. See documentation for supported keys.") + } else { + let suggestions_str = suggestions.join("', '"); + format!("Unknown configuration key: '{key}'. Did you mean: '{suggestions_str}'?") + }; + + Err(ConfigError::Config(error_msg)) + } - Err(ConfigError::Config(error_msg)) + /// Merge another config into this one. + pub fn merge_fields(&mut self, other: Self) { + #(#merge_handlers)* + } } - /// Merge another config into this one. - pub fn merge_fields(&mut self, other: Self) { - #(#merge_handlers)* + fn #find_similar_fn( + unknown: &str, + candidates: &[&str], + max_suggestions: usize, + threshold: f64, + ) -> Vec { + let mut scored: Vec<(String, f64)> = candidates + .iter() + .map(|&candidate| (candidate.to_string(), #calc_similarity_fn(unknown, candidate))) + .filter(|(_, score)| *score >= threshold) + .collect(); + + scored.sort_by(|a, b| b.1.partial_cmp(&a.1).unwrap_or(std::cmp::Ordering::Equal)); + scored.truncate(max_suggestions); + scored.into_iter().map(|(key, _)| key).collect() } - } - - // Include the helper functions in the generated code with unique names - fn #find_similar_fn( - unknown: &str, - candidates: &[&str], - max_suggestions: usize, - threshold: f64, - ) -> Vec { - let mut scored: Vec<(String, f64)> = candidates - .iter() - .map(|&candidate| (candidate.to_string(), #calc_similarity_fn(unknown, candidate))) - .filter(|(_, score)| *score >= threshold) - .collect(); - - scored.sort_by(|a, b| b.1.partial_cmp(&a.1).unwrap_or(std::cmp::Ordering::Equal)); - scored.truncate(max_suggestions); - scored.into_iter().map(|(key, _)| key).collect() - } - fn #calc_similarity_fn(a: &str, b: &str) -> f64 { - let a_chars: Vec = a.chars().collect(); - let b_chars: Vec = b.chars().collect(); - let a_len = a_chars.len(); - let b_len = b_chars.len(); + fn #calc_similarity_fn(a: &str, b: &str) -> f64 { + let a_chars: Vec = a.chars().collect(); + let b_chars: Vec = b.chars().collect(); + let a_len = a_chars.len(); + let b_len = b_chars.len(); - if a_len == 0 && b_len == 0 { - return 1.0; - } - if a_len == 0 || b_len == 0 { - return 0.0; - } + if a_len == 0 && b_len == 0 { + return 1.0; + } + if a_len == 0 || b_len == 0 { + return 0.0; + } - let mut prev_row: Vec = (0..=b_len).collect(); - let mut curr_row = vec![0; b_len + 1]; + let mut prev_row: Vec = (0..=b_len).collect(); + let mut curr_row = vec![0; b_len + 1]; - for i in 1..=a_len { - curr_row[0] = i; - for j in 1..=b_len { - let cost = if a_chars[i - 1] == b_chars[j - 1] { 0 } else { 1 }; - curr_row[j] = (prev_row[j] + 1) - .min(curr_row[j - 1] + 1) - .min(prev_row[j - 1] + cost); + for i in 1..=a_len { + curr_row[0] = i; + for j in 1..=b_len { + let cost = if a_chars[i - 1] == b_chars[j - 1] { 0 } else { 1 }; + curr_row[j] = (prev_row[j] + 1) + .min(curr_row[j - 1] + 1) + .min(prev_row[j - 1] + cost); + } + std::mem::swap(&mut prev_row, &mut curr_row); } - std::mem::swap(&mut prev_row, &mut curr_row); - } - let distance = prev_row[b_len] as f64; - let max_len = a_len.max(b_len) as f64; - 1.0 - (distance / max_len) - } + let distance = prev_row[b_len] as f64; + let max_len = a_len.max(b_len) as f64; + 1.0 - (distance / max_len) + } + }; }; TokenStream::from(expanded) From e7b85578d13bcb0dc425ca6e512a57887b19a88c Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Wed, 25 Mar 2026 23:13:25 +0300 Subject: [PATCH 07/15] ndg-nixdoc: refactor `preceding_doc_comment`; remove dead `ParseNix` variant `preceding_doc_comment` now returns early instead of found `Option` accumulator. Signed-off-by: NotAShelf Change-Id: I097657418769874a5c40e511aae9f3b76a6a6964 --- crates/ndg-nixdoc/src/error.rs | 8 -------- crates/ndg-nixdoc/src/extractor.rs | 26 ++++++++++---------------- 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/crates/ndg-nixdoc/src/error.rs b/crates/ndg-nixdoc/src/error.rs index 0c8c12c9..16582abe 100644 --- a/crates/ndg-nixdoc/src/error.rs +++ b/crates/ndg-nixdoc/src/error.rs @@ -12,12 +12,4 @@ pub enum NixdocError { #[source] source: std::io::Error, }, - - /// The Nix source could not be parsed. - /// - /// Unlike typical parse errors, `rnix` is error-tolerant and always - /// produces a tree. This variant is reserved for cases where the parser - /// signals hard parse errors that make the file unusable. - #[error("failed to parse `{path}` as Nix: {message}")] - ParseNix { path: PathBuf, message: String }, } diff --git a/crates/ndg-nixdoc/src/extractor.rs b/crates/ndg-nixdoc/src/extractor.rs index 75fee7d6..9446416c 100644 --- a/crates/ndg-nixdoc/src/extractor.rs +++ b/crates/ndg-nixdoc/src/extractor.rs @@ -238,30 +238,24 @@ fn preceding_doc_comment(node: &rnix::SyntaxNode) -> Option { // Walk backwards through siblings/tokens from just before this node. // `.skip(1)` skips the node itself (the first element of the iterator). - let mut cursor = node.siblings_with_tokens(Direction::Prev).skip(1); - - let mut found: Option = None; - - for sibling in cursor.by_ref() { - match sibling.kind() { + for token in node.siblings_with_tokens(Direction::Prev).skip(1) { + match token.kind() { TOKEN_WHITESPACE => {}, TOKEN_COMMENT => { - let text = sibling.to_string(); + let text = token.to_string(); if is_doc_comment(&text) { - // Take only the innermost (closest) doc comment. - if found.is_none() { - found = Some(text); - } + return Some(text); } - // Stop after any comment (doc or plain): a plain `# ...` or `/* */` - // separates this binding from any doc comment further up. - break; + + // A plain `# ...` or `/* */` comment separates this binding from any + // doc comment further up; stop searching. + return None; }, - _ => break, + _ => return None, } } - found + None } /// Return `true` when `text` is a Nixdoc block comment (`/** ... */`). From 58f4c5db9f5382d3412f78ec5a8bedafd2259a7d Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Wed, 25 Mar 2026 23:15:35 +0300 Subject: [PATCH 08/15] ndg-templates: cache built-in template map with `OnceLock` Signed-off-by: NotAShelf Change-Id: I735326e305656d38023e03d86938623e6a6a6964 --- crates/ndg-templates/src/lib.rs | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/crates/ndg-templates/src/lib.rs b/crates/ndg-templates/src/lib.rs index 036906ff..f267dab1 100644 --- a/crates/ndg-templates/src/lib.rs +++ b/crates/ndg-templates/src/lib.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::{collections::HashMap, sync::OnceLock}; pub const DEFAULT_TEMPLATE: &str = include_str!("../templates/default.html"); pub const OPTIONS_TEMPLATE: &str = include_str!("../templates/options.html"); @@ -17,17 +17,22 @@ pub const MAIN_JS: &str = include_str!("../templates/main.js"); #[must_use] pub fn all_templates() -> HashMap<&'static str, &'static str> { - let mut templates = HashMap::new(); - templates.insert("default.html", DEFAULT_TEMPLATE); - templates.insert("options.html", OPTIONS_TEMPLATE); - templates.insert("search.html", SEARCH_TEMPLATE); - templates.insert("options_toc.html", OPTIONS_TOC_TEMPLATE); - templates.insert("navbar.html", NAVBAR_TEMPLATE); - templates.insert("footer.html", FOOTER_TEMPLATE); - templates.insert("lib.html", LIB_TEMPLATE); - templates.insert("default.css", DEFAULT_CSS); - templates.insert("search.js", SEARCH_JS); - templates.insert("search-worker.js", SEARCH_WORKER_JS); - templates.insert("main.js", MAIN_JS); - templates + static CACHE: OnceLock> = OnceLock::new(); + CACHE + .get_or_init(|| { + let mut templates = HashMap::new(); + templates.insert("default.html", DEFAULT_TEMPLATE); + templates.insert("options.html", OPTIONS_TEMPLATE); + templates.insert("search.html", SEARCH_TEMPLATE); + templates.insert("options_toc.html", OPTIONS_TOC_TEMPLATE); + templates.insert("navbar.html", NAVBAR_TEMPLATE); + templates.insert("footer.html", FOOTER_TEMPLATE); + templates.insert("lib.html", LIB_TEMPLATE); + templates.insert("default.css", DEFAULT_CSS); + templates.insert("search.js", SEARCH_JS); + templates.insert("search-worker.js", SEARCH_WORKER_JS); + templates.insert("main.js", MAIN_JS); + templates + }) + .clone() } From 16466e5b993497466e054e032d9426cd6770fa39 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Wed, 25 Mar 2026 23:16:11 +0300 Subject: [PATCH 09/15] ndg: handle `--footer` arg in html subcommand Signed-off-by: NotAShelf Change-Id: I6723d68a71e2df2bc023ee45c5d01bde6a6a6964 --- ndg/src/main.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ndg/src/main.rs b/ndg/src/main.rs index 0e762457..05e9e47a 100644 --- a/ndg/src/main.rs +++ b/ndg/src/main.rs @@ -159,7 +159,7 @@ fn merge_cli_into_config(config: &mut Config, cli: &Cli) { stylesheet, script, title, - footer: _, + footer, module_options, options_toc_depth, manpage_urls, @@ -194,6 +194,9 @@ fn merge_cli_into_config(config: &mut Config, cli: &Cli) { if let Some(title) = title { config.title.clone_from(title); } + if let Some(footer) = footer { + config.footer_text.clone_from(footer); + } if let Some(module_options) = module_options { config.module_options = Some(module_options.clone()); } From 3e86c23f0b44963f3ea4437e319b971fcb552fc0 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Wed, 25 Mar 2026 23:18:35 +0300 Subject: [PATCH 10/15] ndg-utils: HTML-escape fallback index output; warn on markdown read failure Signed-off-by: NotAShelf Change-Id: I5d3a63ef92d16e33dc61d34a158a49ac6a6a6964 --- crates/ndg-utils/src/markdown.rs | 9 +++++++-- crates/ndg-utils/src/output.rs | 6 ++++-- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/crates/ndg-utils/src/markdown.rs b/crates/ndg-utils/src/markdown.rs index afaf7287..d9f5373f 100644 --- a/crates/ndg-utils/src/markdown.rs +++ b/crates/ndg-utils/src/markdown.rs @@ -108,7 +108,6 @@ pub struct ProcessedMarkdown { pub frontmatter: Option, } - /// Processes all markdown files in the input directory and returns processed /// data. /// @@ -411,6 +410,12 @@ pub fn extract_page_title(file_path: &Path, html_path: &Path) -> String { ndg_commonmark::utils::extract_markdown_title_and_id(&content) .map_or(default_title, |(title, _)| title) }, - Err(_) => default_title, + Err(e) => { + log::warn!( + "Failed to read {} for title extraction: {e}", + file_path.display() + ); + default_title + }, } } diff --git a/crates/ndg-utils/src/output.rs b/crates/ndg-utils/src/output.rs index 70a76585..9c50803f 100644 --- a/crates/ndg-utils/src/output.rs +++ b/crates/ndg-utils/src/output.rs @@ -52,8 +52,10 @@ pub fn create_fallback_index( let _ = writeln!( file_list, "
  • {}
  • ", - html_path.to_string_lossy(), - page_title + html_escape::encode_double_quoted_attribute( + &html_path.to_string_lossy() + ), + html_escape::encode_text(&page_title) ); } } From 1c18829d0708100929fbf618b5d3a974df541db3 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Wed, 25 Mar 2026 23:21:24 +0300 Subject: [PATCH 11/15] ndg-manpage: fix declarations parsing & escape bounds check; restore formatting Signed-off-by: NotAShelf Change-Id: I7e4f1328ae3ce7ea8414472db6a649b56a6a6964 --- crates/ndg-manpage/src/options.rs | 9 ++++++--- crates/ndg-manpage/tests/manpage_output.rs | 5 +++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/ndg-manpage/src/options.rs b/crates/ndg-manpage/src/options.rs index 09d5d6d3..82ffae08 100644 --- a/crates/ndg-manpage/src/options.rs +++ b/crates/ndg-manpage/src/options.rs @@ -389,7 +389,9 @@ fn parse_option( } // File where option is declared - if let Some(Value::String(file)) = option_data.get("declarations") { + if let Some(Value::Array(files)) = option_data.get("declarations") + && let Some(Value::String(file)) = files.first() + { option.declared_in = Some(file.clone()); } @@ -447,7 +449,8 @@ fn process_description(text: &str) -> String { let escaped = escape_non_macro_lines(&with_admonitions); // Preserve explicit paragraph breaks so list items do not merge - escaped.replace("\n\n", "\n.br\n") + let result = escaped.replace("\n\n", "\n.br\n"); + restore_formatting(&result) } /// Preserve existing troff formatting codes so they don't get double-escaped @@ -564,7 +567,7 @@ fn selective_man_escape(text: &str) -> String { i += 2; // If it's a special escape, process accordingly - if chars[i - 1] == '(' && i + 2 <= chars.len() { + if chars[i - 1] == '(' && i + 1 < chars.len() { result.push(chars[i]); result.push(chars[i + 1]); i += 2; diff --git a/crates/ndg-manpage/tests/manpage_output.rs b/crates/ndg-manpage/tests/manpage_output.rs index f48e645f..fdd9e345 100644 --- a/crates/ndg-manpage/tests/manpage_output.rs +++ b/crates/ndg-manpage/tests/manpage_output.rs @@ -12,17 +12,18 @@ fn test_manpage_declared_by_formatting() { let options_path = temp_dir.path().join("options.json"); let output_path = temp_dir.path().join("out.5"); + // NixOS options JSON always uses an array for `declarations` let options = json!({ "services.test": { "type": "string", "description": "Test option", - "declarations": "modules/test/module.nix", + "declarations": ["modules/test/module.nix"], "declarationURL": "https://example.com/test" }, "services.no_url": { "type": "string", "description": "Another option", - "declarations": "modules/other/module.nix" + "declarations": ["modules/other/module.nix"] } }); From 5da3b44429e67de950e918ea25d323d213e40484 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Thu, 26 Mar 2026 22:01:34 +0300 Subject: [PATCH 12/15] ndg-html: preserve option sort order with `IndexMap`; HTML-escape names Signed-off-by: NotAShelf Change-Id: I41bbede677512d74b556baa4c6db3fc46a6a6964 --- Cargo.lock | 1 + Cargo.toml | 1 + crates/ndg-html/Cargo.toml | 1 + crates/ndg-html/src/options.rs | 54 ++++------ crates/ndg-html/src/search.rs | 1 - crates/ndg-html/src/template.rs | 100 ++++++------------ crates/ndg-html/tests/html_template.rs | 11 +- .../ndg-html/tests/search_path_resolution.rs | 8 +- 8 files changed, 67 insertions(+), 110 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b0858f6b..57fb15bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1907,6 +1907,7 @@ dependencies = [ "color-eyre", "grass", "html-escape", + "indexmap 2.13.0", "log", "ndg-commonmark", "ndg-config", diff --git a/Cargo.toml b/Cargo.toml index c65a71ec..20db412a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -51,6 +51,7 @@ walkdir = "2.5.0" html-escape = "0.2.13" html5ever = "0.38.0" +indexmap = "2.13.0" kuchikikiki = "0.9.2" lightningcss = { version = "1.0.0-alpha.71", default-features = false } markup5ever = "0.38.0" diff --git a/crates/ndg-html/Cargo.toml b/crates/ndg-html/Cargo.toml index ab55d0bc..89558edd 100644 --- a/crates/ndg-html/Cargo.toml +++ b/crates/ndg-html/Cargo.toml @@ -25,6 +25,7 @@ ndg-templates.workspace = true ndg-utils.workspace = true color-eyre.workspace = true +indexmap.workspace = true grass.workspace = true html-escape.workspace = true log.workspace = true diff --git a/crates/ndg-html/src/options.rs b/crates/ndg-html/src/options.rs index d79932e2..a63f689f 100644 --- a/crates/ndg-html/src/options.rs +++ b/crates/ndg-html/src/options.rs @@ -5,6 +5,7 @@ use std::{ }; use color_eyre::eyre::{Context, Result}; +use indexmap::IndexMap; use log::debug; use ndg_config::Config; use ndg_manpage::types::NixOption; @@ -168,43 +169,26 @@ pub fn process_options(config: &Config, options_path: &Path) -> Result<()> { } } - // Sort options by priority (enable > package > other) - let mut sorted_options: Vec<_> = options.into_iter().collect(); - sorted_options.sort_by(|(name_a, _), (name_b, _)| { - // Calculate priority for name_a - let priority_a = if name_a.starts_with("enable") { - 0 - } else if name_a.starts_with("package") { - 1 - } else { - 2 - }; - - // Calculate priority for name_b - let priority_b = if name_b.starts_with("enable") { - 0 - } else if name_b.starts_with("package") { - 1 - } else { - 2 + // Sort options by priority (enable > package > other), with secondary + // alphabetical sort within each priority group. + let mut sorted: Vec<_> = options.into_iter().collect(); + sorted.sort_by(|(name_a, _), (name_b, _)| { + let priority = |name: &str| { + if name.starts_with("enable") { + 0 + } else if name.starts_with("package") { + 1 + } else { + 2 + } }; - - // Compare by priority first, then by name - priority_a.cmp(&priority_b).then_with(|| name_a.cmp(name_b)) + priority(name_a) + .cmp(&priority(name_b)) + .then_with(|| name_a.cmp(name_b)) }); - // Convert back to HashMap preserving the new order - let customized_options = sorted_options - .iter() - .map(|(key, opt)| { - let option_clone = opt.clone(); - (key.clone(), option_clone) - }) - .map(|(key, mut opt)| { - opt.name = html_escape::encode_text(&opt.name).to_string(); - (key, opt) - }) - .collect(); + let customized_options: IndexMap = + sorted.into_iter().collect(); // Render options page let html = template::render_options(config, &customized_options)?; @@ -247,9 +231,7 @@ fn format_location( let path_str = path.as_str(); if path_str.starts_with('/') { - // Local filesystem path handling let url = format!("file://{path_str}"); - if path_str.contains("nixops") && path_str.contains("/nix/") { let suffix_index = path_str.find("/nix/").map_or(0, |i| i + 5); let suffix = &path_str[suffix_index..]; diff --git a/crates/ndg-html/src/search.rs b/crates/ndg-html/src/search.rs index 37c1bef0..25d69571 100644 --- a/crates/ndg-html/src/search.rs +++ b/crates/ndg-html/src/search.rs @@ -1,5 +1,4 @@ use std::{ - clone::Clone, collections::{HashMap, HashSet}, fs, path::PathBuf, diff --git a/crates/ndg-html/src/template.rs b/crates/ndg-html/src/template.rs index e6c72afe..9de57647 100644 --- a/crates/ndg-html/src/template.rs +++ b/crates/ndg-html/src/template.rs @@ -3,12 +3,12 @@ use std::{ fmt::Write, fs, path::Path, - string::String, sync::{LazyLock, RwLock}, }; use color_eyre::eyre::{Context, Result, bail}; use html_escape::encode_text; +use indexmap::IndexMap; use ndg_commonmark::Header; use ndg_config::{Config, sidebar::SidebarOrdering}; use ndg_manpage::types::NixOption; @@ -466,7 +466,7 @@ fn resolve_doc_template( )] pub fn render_options( config: &Config, - options: &HashMap, + options: &IndexMap, ) -> Result { // Load templates with caching let options_template = @@ -598,7 +598,7 @@ pub fn render_lib( /// Generate specialized TOC for options page fn generate_options_toc( - options: &HashMap, + options: &IndexMap, config: &Config, tera: &Tera, ) -> Result { @@ -1006,7 +1006,6 @@ struct NavItem { number: Option, } - /// Generate the document navigation HTML fn generate_doc_nav(config: &Config, current_file_rel_path: &Path) -> String { let mut doc_nav = String::new(); @@ -1066,7 +1065,8 @@ fn generate_doc_nav(config: &Config, current_file_rel_path: &Path) -> String { format!("{}{}", root_prefix, html_path.to_string_lossy()); // Extract page title - let page_title = ndg_utils::markdown::extract_page_title(path, &html_path); + let page_title = + ndg_utils::markdown::extract_page_title(path, &html_path); // Apply sidebar configuration if available let (display_title, position) = @@ -1142,7 +1142,8 @@ fn generate_doc_nav(config: &Config, current_file_rel_path: &Path) -> String { let target_path = format!("{}{}", root_prefix, html_path.to_string_lossy()); - let page_title = ndg_utils::markdown::extract_page_title(path, &html_path); + let page_title = + ndg_utils::markdown::extract_page_title(path, &html_path); // Apply sidebar configuration to special files if available let display_title = if let Some(sidebar_config) = &config.sidebar { @@ -1357,19 +1358,14 @@ fn generate_toc(headers: &[Header]) -> String { } /// Generate the options HTML content -fn generate_options_html(options: &HashMap) -> String { - let mut options_html = String::with_capacity(options.len() * 500); // FIXME: Rough estimate for capacity - - // Sort options by name - let mut option_keys: Vec<_> = options.keys().collect(); - option_keys.sort(); +fn generate_options_html(options: &IndexMap) -> String { + let mut options_html = String::with_capacity(options.len() * 500); // rough capacity estimate, average ~500 bytes per option - for key in option_keys { - let option = &options[key]; + // Iterate in the order established by process_options, i.e., priority sort. + for option in options.values() { let option_id = format!("option-{}", option.name.replace('.', "-")); - // Open option container with ID for direct linking - // Writing to String is infallible + // Open option container with ID for direct linking. let _ = writeln!(options_html, "
    "); // Option name with anchor link and copy button @@ -1379,7 +1375,8 @@ fn generate_options_html(options: &HashMap) -> String { class=\"option-anchor\">{}\n \n Link copied!\n \n", - option_id, option.name + option_id, + encode_text(&option.name) ); // Option metadata (internal/readOnly) @@ -1476,71 +1473,44 @@ fn add_default_value(html: &mut String, option: &NixOption) { /// Add example value to options HTML fn add_example_value(html: &mut String, option: &NixOption) { if let Some(example_text) = &option.example_text { - // Process the example text to preserve code formatting - if example_text.contains('\n') { - // Multi-line examples - preserve formatting with pre/code - // Process special characters to ensure valid HTML - let safe_example = example_text.replace('<', "<").replace('>', ">"); - - // Remove backticks if they're surrounding the entire content (from - // literalExpression) - let trimmed_example = if safe_example.starts_with('`') - && safe_example.ends_with('`') - && safe_example.len() > 2 - { - &safe_example[1..safe_example.len() - 1] - } else { - &safe_example - }; + // Strip surrounding backticks (literalExpression wrapper) before escaping, + // so the backtick positions are checked on the original text. + let inner: &str = if example_text.starts_with('`') + && example_text.ends_with('`') + && example_text.len() > 2 + { + &example_text[1..example_text.len() - 1] + } else { + example_text + }; - // Writing to String is infallible + let safe = encode_text(inner); + + if inner.contains('\n') { let _ = writeln!( html, "
    Example: \ -
    {trimmed_example}
    " +
    {safe}
    " ); } else { - // Check if this is already a code block (surrounded by backticks) - if example_text.starts_with('`') - && example_text.ends_with('`') - && example_text.len() > 2 - { - // This is inline code - extract the content and properly escape it - let code_content = &example_text[1..example_text.len() - 1]; - let safe_content = - code_content.replace('<', "<").replace('>', ">"); - let _ = writeln!( - html, - "
    Example: \ - {safe_content}
    " - ); - } else { - // Regular inline example - still needs escaping - let safe_example = - example_text.replace('<', "<").replace('>', ">"); - let _ = writeln!( - html, - "
    Example: \ - {safe_example}
    " - ); - } + let _ = writeln!( + html, + "
    Example: {safe}
    " + ); } } else if let Some(example_val) = &option.example { let example_str = example_val.to_string(); - let safe_example = example_str.replace('<', "<").replace('>', ">"); + let safe = encode_text(&example_str); if example_str.contains('\n') { - // Multi-line JSON examples need special handling let _ = writeln!( html, "
    Example: \ -
    {safe_example}
    " +
    {safe}
    " ); } else { - // Single-line JSON examples let _ = writeln!( html, - "
    Example: \ - {safe_example}
    " + "
    Example: {safe}
    " ); } } diff --git a/crates/ndg-html/tests/html_template.rs b/crates/ndg-html/tests/html_template.rs index b9df5e4d..00a29e64 100644 --- a/crates/ndg-html/tests/html_template.rs +++ b/crates/ndg-html/tests/html_template.rs @@ -1,6 +1,7 @@ #![allow(clippy::expect_used, reason = "Fine in tests")] use std::{collections::HashMap, fs, path::Path}; +use indexmap::IndexMap; use ndg_commonmark::{Header, MarkdownOptions, MarkdownProcessor}; use ndg_config::{Config, search::SearchConfig, sidebar::SidebarConfig}; use ndg_html::template; @@ -69,7 +70,7 @@ fn render_basic_page_renders_html() { fn render_options_page_includes_options() { let mut config = minimal_config(); config.module_options = Some("dummy.json".into()); - let mut options = HashMap::new(); + let mut options = IndexMap::new(); options.insert( "foo.bar".to_string(), create_basic_option("foo.bar", "desc"), @@ -84,7 +85,7 @@ fn render_options_page_includes_options() { fn render_options_page_renders_description() { let mut config = minimal_config(); config.module_options = Some("dummy.json".into()); - let mut options = HashMap::new(); + let mut options = IndexMap::new(); options.insert( "foo.bar".to_string(), create_detailed_option( @@ -173,7 +174,7 @@ fn render_page_with_headers_toc() { fn render_options_page_with_multiple_options() { let mut config = minimal_config(); config.module_options = Some("dummy.json".into()); - let mut options = HashMap::new(); + let mut options = IndexMap::new(); options.insert( "foo.bar".to_string(), create_basic_option("foo.bar", "desc1"), @@ -249,7 +250,7 @@ fn render_page_contains_footer_html() { fn render_options_page_contains_navbar() { let mut config = minimal_config(); config.module_options = Some("dummy.json".into()); - let mut options = HashMap::new(); + let mut options = IndexMap::new(); options.insert( "test.option".to_string(), create_basic_option("test.option", "Test option"), @@ -267,7 +268,7 @@ fn render_options_page_contains_footer() { let mut config = minimal_config(); config.module_options = Some("dummy.json".into()); config.footer_text = "Custom Footer Text".to_string(); - let mut options = HashMap::new(); + let mut options = IndexMap::new(); options.insert( "test.option".to_string(), create_basic_option("test.option", "Test option"), diff --git a/crates/ndg-html/tests/search_path_resolution.rs b/crates/ndg-html/tests/search_path_resolution.rs index a846fdfb..f52b6894 100644 --- a/crates/ndg-html/tests/search_path_resolution.rs +++ b/crates/ndg-html/tests/search_path_resolution.rs @@ -200,9 +200,11 @@ This file should be transitively included in `main.html` let processor = Some(create_processor(&config, None)); - // Populate config.included_files as a side effect of processing markdown files - let _processed_files = process_markdown_files(&mut config, processor.as_ref()) - .expect("Failed to process markdown files"); + // Populate config.included_files as a side effect of processing markdown + // files + let _processed_files = + process_markdown_files(&mut config, processor.as_ref()) + .expect("Failed to process markdown files"); let all_markdown_files = collect_markdown_files(&input_dir); From 9ee25b6928785fe050336eae8ed7e89622fab25a Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Thu, 26 Mar 2026 22:05:54 +0300 Subject: [PATCH 13/15] ndg-config: fix TOML template scoping; tweak merge semantics & sidebar guards Signed-off-by: NotAShelf Change-Id: I4ef4f4d17e999d8e6b90de015d8b83a46a6a6964 --- crates/ndg-config/src/config.rs | 94 ++++++++++++++---------------- crates/ndg-config/src/sidebar.rs | 15 +++-- crates/ndg-config/src/templates.rs | 16 ++--- 3 files changed, 63 insertions(+), 62 deletions(-) diff --git a/crates/ndg-config/src/config.rs b/crates/ndg-config/src/config.rs index e8169c69..ff7b125d 100644 --- a/crates/ndg-config/src/config.rs +++ b/crates/ndg-config/src/config.rs @@ -2,7 +2,6 @@ use std::{ collections::HashMap, fs, path::{Path, PathBuf}, - sync::OnceLock, }; use ndg_macros::Configurable; @@ -449,22 +448,18 @@ impl Config { "The 'opengraph' config field is deprecated. Use 'meta.opengraph' \ instead." ); - if let Some(ref mut og) = self.opengraph { - og.extend(other_og); - } else { - self.opengraph = Some(other_og); - } + + // Later value wins + self.opengraph = Some(other_og); } if let Some(other_meta) = other.meta_tags.take() { log::warn!( "The 'meta_tags' config field is deprecated. Use 'meta.tags' \ instead." ); - if let Some(ref mut meta) = self.meta_tags { - meta.extend(other_meta); - } else { - self.meta_tags = Some(other_meta); - } + + // Later value wins + self.meta_tags = Some(other_meta); } } @@ -530,52 +525,51 @@ impl Config { None } - /// Search for config files in common locations + /// Search for config files in common locations. + /// + /// This performs a fresh filesystem lookup on every call, and the result is + /// not cached so that tests calling from different working directories each + /// see the correct config for their directory. #[must_use] pub fn find_config_file() -> Option { - static RESULT: OnceLock> = OnceLock::new(); - RESULT - .get_or_init(|| { - let config_filenames = [ - "ndg.toml", - "ndg.json", - ".ndg.toml", - ".ndg.json", - ".config/ndg.toml", - ".config/ndg.json", - ]; - - let current_dir = std::env::current_dir().ok()?; - for filename in &config_filenames { - let config_path = current_dir.join(filename); - if config_path.exists() { - return Some(config_path); - } - } + let config_filenames = [ + "ndg.toml", + "ndg.json", + ".ndg.toml", + ".ndg.json", + ".config/ndg.toml", + ".config/ndg.json", + ]; + + let current_dir = std::env::current_dir().ok()?; + for filename in &config_filenames { + let config_path = current_dir.join(filename); + if config_path.exists() { + return Some(config_path); + } + } - if let Ok(xdg_config_home) = std::env::var("XDG_CONFIG_HOME") { - let xdg_config_dir = PathBuf::from(xdg_config_home); - for filename in &["ndg.toml", "ndg.json"] { - let config_path = xdg_config_dir.join(filename); - if config_path.exists() { - return Some(config_path); - } - } + if let Ok(xdg_config_home) = std::env::var("XDG_CONFIG_HOME") { + let xdg_config_dir = PathBuf::from(xdg_config_home).join("ndg"); + for filename in &["config.toml", "config.json"] { + let config_path = xdg_config_dir.join(filename); + if config_path.exists() { + return Some(config_path); } + } + } - if let Ok(home) = std::env::var("HOME") { - let home_config_dir = PathBuf::from(home).join(".config").join("ndg"); - for filename in &["config.toml", "config.json"] { - let config_path = home_config_dir.join(filename); - if config_path.exists() { - return Some(config_path); - } - } + if let Ok(home) = std::env::var("HOME") { + let home_config_dir = PathBuf::from(home).join(".config").join("ndg"); + for filename in &["config.toml", "config.json"] { + let config_path = home_config_dir.join(filename); + if config_path.exists() { + return Some(config_path); } + } + } - None - }) - .clone() + None } /// Validate all paths specified in the configuration diff --git a/crates/ndg-config/src/sidebar.rs b/crates/ndg-config/src/sidebar.rs index 5304f461..347b1166 100644 --- a/crates/ndg-config/src/sidebar.rs +++ b/crates/ndg-config/src/sidebar.rs @@ -384,6 +384,11 @@ impl SidebarMatch { /// always be validated/compiled via `SidebarConfig::validate()`. #[must_use] pub fn matches(&self, path_str: &str, title_str: &str) -> bool { + // A rule with no conditions matches nothing; require at least one filter. + if self.path.is_none() && self.title.is_none() { + return false; + } + // Check path matching if let Some(ref path_match) = self.path { // Check exact path match @@ -627,6 +632,11 @@ impl OptionsMatch { /// always be validated/compiled via `OptionsConfig::validate()`. #[must_use] pub fn matches(&self, option_name: &str) -> bool { + // A rule with no conditions matches nothing. + if self.name.is_none() { + return false; + } + // Check name matching if let Some(ref name_match) = self.name { // Check exact name match @@ -673,10 +683,7 @@ impl OptionsMatch { /// Check if this option should be hidden from the TOC. #[must_use] pub const fn is_hidden(&self) -> bool { - match self.hidden { - Some(hidden) => hidden, - None => false, - } + matches!(self.hidden, Some(true)) } } diff --git a/crates/ndg-config/src/templates.rs b/crates/ndg-config/src/templates.rs index b9b14c7b..801e3850 100644 --- a/crates/ndg-config/src/templates.rs +++ b/crates/ndg-config/src/templates.rs @@ -68,14 +68,6 @@ footer_text = "Generated with ndg" # Whether to generate anchors for headings generate_anchors = true -# Search configuration -[search] -# Whether to generate a search index -enable = true - -# Maximum heading level to index -max_heading_level = 3 - # Depth of parent categories in options TOC options_toc_depth = 2 @@ -94,6 +86,14 @@ revision = "main" # Additional meta tags to inject into the HTML head (example: { description = "Docs", keywords = "nix,docs" }) # meta_tags = { description = "Documentation for My Project", keywords = "nix,docs,example" } +# Search configuration +[search] +# Whether to generate a search index +enable = true + +# Maximum heading level to index +max_heading_level = 3 + # Sidebar configuration # [sidebar] # Enable numbering for sidebar items From b0d8ca84895bd5d869468959b1b8340e4af48ffe Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Thu, 26 Mar 2026 22:27:03 +0300 Subject: [PATCH 14/15] ndg-html: sanitize option IDs outside of Tera context Always cleaner this way. Signed-off-by: NotAShelf Change-Id: I26280424e3b9ae3b5214505d2792f8166a6a6964 --- crates/ndg-html/src/template.rs | 73 +++++++-- crates/ndg-html/tests/html_template.rs | 138 ++++++++++++++++++ .../ndg-templates/templates/options_toc.html | 6 +- 3 files changed, 204 insertions(+), 13 deletions(-) diff --git a/crates/ndg-html/src/template.rs b/crates/ndg-html/src/template.rs index 9de57647..442e953d 100644 --- a/crates/ndg-html/src/template.rs +++ b/crates/ndg-html/src/template.rs @@ -679,8 +679,19 @@ fn generate_options_toc( let option_value = tera::to_value({ let mut map = tera::Map::new(); - map.insert("name".to_string(), tera::to_value(&option.name)?); - map.insert("display_name".to_string(), tera::to_value(display_name)?); + // HTML-escape display strings: Tera::default() has no auto-escaping. + map.insert( + "name".to_string(), + tera::to_value(encode_text(&option.name).as_ref())?, + ); + map.insert( + "id".to_string(), + tera::to_value(sanitize_option_id(&option.name))?, + ); + map.insert( + "display_name".to_string(), + tera::to_value(encode_text(display_name).as_ref())?, + ); map.insert("internal".to_string(), tera::to_value(option.internal)?); map.insert("read_only".to_string(), tera::to_value(option.read_only)?); @@ -703,10 +714,13 @@ fn generate_options_toc( .map(String::as_str) .unwrap_or(parent); - category.insert("name".to_string(), tera::to_value(parent)?); + category.insert( + "name".to_string(), + tera::to_value(encode_text(parent).as_ref())?, + ); category.insert( "display_name".to_string(), - tera::to_value(category_display_name)?, + tera::to_value(encode_text(category_display_name).as_ref())?, ); category.insert("count".to_string(), tera::to_value(opts.len())?); @@ -714,7 +728,14 @@ fn generate_options_toc( if let Some(parent_option) = direct_parent_options.get(parent) { let parent_option_value = tera::to_value({ let mut map = tera::Map::new(); - map.insert("name".to_string(), tera::to_value(&parent_option.name)?); + map.insert( + "name".to_string(), + tera::to_value(encode_text(&parent_option.name).as_ref())?, + ); + map.insert( + "id".to_string(), + tera::to_value(sanitize_option_id(&parent_option.name))?, + ); map.insert( "internal".to_string(), tera::to_value(parent_option.internal)?, @@ -757,8 +778,18 @@ fn generate_options_toc( let child_value = tera::to_value({ let mut map = tera::Map::new(); - map.insert("name".to_string(), tera::to_value(&option.name)?); - map.insert("display_name".to_string(), tera::to_value(display_name)?); + map.insert( + "name".to_string(), + tera::to_value(encode_text(&option.name).as_ref())?, + ); + map.insert( + "id".to_string(), + tera::to_value(sanitize_option_id(&option.name))?, + ); + map.insert( + "display_name".to_string(), + tera::to_value(encode_text(display_name).as_ref())?, + ); map.insert("internal".to_string(), tera::to_value(option.internal)?); map .insert("read_only".to_string(), tera::to_value(option.read_only)?); @@ -1357,13 +1388,33 @@ fn generate_toc(headers: &[Header]) -> String { toc } +/// Produce a safe HTML `id` / URL fragment from an option name. +/// +/// Dots are the canonical separator in NixOS option names, but names may also +/// contain `` placeholders (angle brackets) and other characters that +/// are invalid or dangerous in HTML `id` attributes and URL fragments. Replace +/// every character that is not ASCII alphanumeric, `-`, or `_` with `-`. +fn sanitize_option_id(name: &str) -> String { + let sanitized: String = name + .chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c == '-' || c == '_' { + c + } else { + '-' + } + }) + .collect(); + format!("option-{sanitized}") +} + /// Generate the options HTML content fn generate_options_html(options: &IndexMap) -> String { let mut options_html = String::with_capacity(options.len() * 500); // rough capacity estimate, average ~500 bytes per option // Iterate in the order established by process_options, i.e., priority sort. for option in options.values() { - let option_id = format!("option-{}", option.name.replace('.', "-")); + let option_id = sanitize_option_id(&option.name); // Open option container with ID for direct linking. let _ = writeln!(options_html, "
    "); @@ -1401,7 +1452,7 @@ fn generate_options_html(options: &IndexMap) -> String { let _ = writeln!( options_html, "
    Type: {}
    ", - option.type_name + encode_text(&option.type_name) ); // Option description @@ -1421,10 +1472,12 @@ fn generate_options_html(options: &IndexMap) -> String { if let Some(declared_in) = &option.declared_in { // Writing to String is infallible if let Some(url) = &option.declared_in_url { + let safe_url = html_escape::encode_double_quoted_attribute(url); let _ = writeln!( options_html, "
    Declared in: {declared_in}
    " + href=\"{safe_url}\" \ + target=\"_blank\">{declared_in}
    " ); } else { let _ = writeln!( diff --git a/crates/ndg-html/tests/html_template.rs b/crates/ndg-html/tests/html_template.rs index 00a29e64..31d436d7 100644 --- a/crates/ndg-html/tests/html_template.rs +++ b/crates/ndg-html/tests/html_template.rs @@ -906,3 +906,141 @@ fn render_page_builtin_vars_take_precedence_over_user_vars() { "user var must not shadow built-in" ); } + +// NixOS option names commonly contain `` as a placeholder component, +// e.g. `services.nginx.virtualHosts..serverName`. The `<` and `>` must +// not appear raw inside an HTML `id` attribute or `href` fragment. +#[test] +fn render_options_id_attribute_is_safe_for_angle_bracket_names() { + let mut config = minimal_config(); + config.module_options = Some("dummy.json".into()); + let name = "services.nginx.virtualHosts..serverName"; + let mut options = IndexMap::new(); + options.insert(name.to_string(), create_basic_option(name, "desc")); + + let html = template::render_options(&config, &options).expect("render"); + + // Raw `` must never appear inside an id="..." attribute value. + // If it did the browser would parse `` as an HTML tag, breaking the + // page structure. + assert!( + !html.contains("id=\"option-services-nginx-virtualHosts-"), + "raw '<' must not appear inside an id attribute" + ); + assert!( + !html.contains("href=\"#option-services-nginx-virtualHosts-"), + "raw '<' must not appear inside an href fragment" + ); + + // The id and matching href must both be present so the anchor still works. + // The sanitized form replaces all non-alphanumeric chars except `-`/`_` + // with `-`, so `.` and `<`/`>` all become `-`. + let expected_id = "option-services-nginx-virtualHosts--name--serverName"; + assert!( + html.contains(&format!("id=\"{expected_id}\"")), + "sanitized id must be present: {expected_id}" + ); + assert!( + html.contains(&format!("href=\"#{expected_id}\"")), + "TOC href must match the sanitized id: #{expected_id}" + ); + + // The display text must still show the real name, properly HTML-escaped. + assert!( + html.contains("<name>"), + "display text must HTML-escape angle brackets" + ); + assert!( + !html.contains(""), + "raw unescaped tag must not appear anywhere in output" + ); +} + +// `type_name` is rendered inside a `` element. Values like +// `null or (submodule)` are benign, but the field is free-form and must be +// escaped to prevent injection. +#[test] +fn render_options_type_name_is_html_escaped() { + let mut config = minimal_config(); + config.module_options = Some("dummy.json".into()); + let mut options = IndexMap::new(); + options.insert("foo.bar".to_string(), NixOption { + name: "foo.bar".to_string(), + description: "desc".to_string(), + type_name: "null or ".to_string(), + ..Default::default() + }); + + let html = template::render_options(&config, &options).expect("render"); + + assert!( + !html.contains("`, so angle brackets and +// ampersands must be preserved as-is. +#[test] +fn render_options_declared_in_preserves_angle_brackets() { + let mut config = minimal_config(); + config.module_options = Some("dummy.json".into()); + let mut options = IndexMap::new(); + options.insert("foo.bar".to_string(), NixOption { + name: "foo.bar".to_string(), + description: "desc".to_string(), + declared_in: Some("/foo/bar.nix".to_string()), + ..Default::default() + }); + + let html = template::render_options(&config, &options).expect("render"); + + assert!( + html.contains(""), + "raw '' must appear in declared_in output" + ); + assert!( + !html.contains("<modules>"), + "angle brackets in declared_in must not be escaped" + ); +} + +// When `declared_in_url` contains an ampersand (valid in URLs, but must be +// `&` inside an HTML attribute), the href must be properly escaped. +#[test] +fn render_options_declared_in_url_ampersand_is_escaped_in_attribute() { + let mut config = minimal_config(); + config.module_options = Some("dummy.json".into()); + let mut options = IndexMap::new(); + options.insert("foo.bar".to_string(), NixOption { + name: "foo.bar".to_string(), + description: "desc".to_string(), + declared_in: Some("foo/bar.nix".to_string()), + declared_in_url: Some( + "https://example.com/source?file=foo.nix&line=42".to_string(), + ), + ..Default::default() + }); + + let html = template::render_options(&config, &options).expect("render"); + + // Raw `&line=` inside an href attribute is invalid HTML; must be + // `&line=`. + assert!( + !html.contains("href=\"https://example.com/source?file=foo.nix&line="), + "raw '&' inside href attribute must not appear" + ); + assert!( + html.contains("&line=42"), + "ampersand in href must be escaped to &" + ); +} diff --git a/crates/ndg-templates/templates/options_toc.html b/crates/ndg-templates/templates/options_toc.html index 7fac6102..fd3eca14 100644 --- a/crates/ndg-templates/templates/options_toc.html +++ b/crates/ndg-templates/templates/options_toc.html @@ -2,7 +2,7 @@ {# Single options first #} {% for option in single_options %}
  • - + {{ option.name }} {% if option.internal %}internal{% endif %} {% if option.read_only %}read-only{% endif %} @@ -21,7 +21,7 @@