Merge pull request #80 from feel-co/notashelf/push-zymtmkqlsntk

ndg: filter out included files in search index

Author: NotAShelf

Cargo.lock

@@ -26,7 +26,7 @@ clap = { version = "4.5.53", default-features = false, features = [
 clap_complete = "4.5.61"
 clap_mangen = "0.2.31"
 color-eyre = "0.6.5"
-comrak = { version = "0.48.0", default-features = false, features = [ "syntect" ] }
+comrak = { version = "0.49.0", default-features = false, features = ["syntect"] }
 env_logger = "0.11.8"
 fs_extra = "1.3.0"
 grass = { version = "0.13.4", default-features = false }

Cargo.toml

@@ -1,13 +1,12 @@
 use std::{
   collections::{HashMap, HashSet},
   fs,
-  path::{Path, PathBuf},
+  path::PathBuf,
   sync::OnceLock,
 };
 
 use color_eyre::eyre::{Context, Result};
 use log::info;
-use ndg_commonmark::utils::slugify;
 use rayon::prelude::*;
 use regex::Regex;
 use serde::{Deserialize, Serialize};
@@ -68,7 +67,12 @@ fn tokenize(text: &str) -> Vec<String> {
   tokens.into_iter().collect()
 }
 
-/// Generate search index from markdown files
+/// Generate search index from standalone markdown files.
+///
+/// This function only processes markdown files that will be rendered as
+/// standalone HTML pages. Files that are included in other documents via
+/// `{=include=}` directives should not be passed to this function - their
+/// content is already indexed as part of the parent document.
 ///
 /// # Errors
 ///
@@ -95,7 +99,7 @@ pub fn generate_search_index(
   let mut search_index = SearchIndex::new();
   let mut doc_id = 0;
 
-  // Process markdown files in parallel if available and input_dir is provided
+  // Process standalone markdown files in parallel
   if !markdown_files.is_empty()
     && let Some(ref input_dir) = config.input_dir
   {
@@ -109,16 +113,17 @@ pub fn generate_search_index(
           )
         })?;
 
-        let (title, id) = extract_title_and_id(&content).unwrap_or_else(|| {
-          (
-            file_path
-              .file_stem()
-              .unwrap_or_default()
-              .to_string_lossy()
-              .to_string(),
-            None,
-          )
-        });
+        let (title, _id) =
+          extract_title_and_id(&content).unwrap_or_else(|| {
+            (
+              file_path
+                .file_stem()
+                .unwrap_or_default()
+                .to_string_lossy()
+                .to_string(),
+              None,
+            )
+          });
 
         let plain_text = crate::utils::html::content_to_plaintext(&content);
 
@@ -130,30 +135,10 @@ pub fn generate_search_index(
             )
           })?;
 
-        let mut output_path = config.included_files.get(rel_path).map_or_else(
-          || rel_path.to_owned(),
-          |mut includer| {
-            // find the root document that transitively includes this file
-            // NOTE: it'll be more efficient to resolve this in
-            // collect_included_files() but with the way it works
-            // right now, it's not easy to
-            while let Some(parent) = config.included_files.get(includer) {
-              includer = parent;
-            }
-            includer.to_owned()
-          },
-        );
-        output_path.set_extension("html");
-
-        let path = if config.included_files.contains_key(rel_path) {
-          format!(
-            "{}#{}",
-            output_path.to_string_lossy(),
-            id.unwrap_or_else(|| slugify(&title))
-          )
-        } else {
-          output_path.to_string_lossy().to_string()
-        };
+        // Convert markdown path to HTML path
+        let mut html_path = rel_path.to_path_buf();
+        html_path.set_extension("html");
+        let path = html_path.to_string_lossy().to_string();
 
         let tokens = tokenize(&plain_text);
         let title_tokens = tokenize(&title);

ndg/src/html/search.rs

@@ -1,4 +1,4 @@
-use std::fs;
+use std::{fs, path::PathBuf};
 
 use color_eyre::eyre::{Context, Result};
 use log::{LevelFilter, info};
@@ -156,7 +156,26 @@ fn generate_documentation(config: &mut Config) -> Result<()> {
   // Generate search index if enabled, regardless of whether there are markdown
   // files
   if config.generate_search {
-    html::search::generate_search_index(config, &markdown_files)?;
+    // Filter out included files - they should not appear as standalone entries
+    // in search results. Their content is indexed as part of the parent
+    // document.
+    let searchable_files: Vec<PathBuf> =
+      if let Some(ref input_dir) = config.input_dir {
+        markdown_files
+          .iter()
+          .filter(|file| {
+            file
+              .strip_prefix(input_dir)
+              .ok()
+              .is_none_or(|rel| !config.included_files.contains_key(rel))
+          })
+          .cloned()
+          .collect()
+      } else {
+        markdown_files
+      };
+
+    html::search::generate_search_index(config, &searchable_files)?;
   }
 
   // Copy assets

ndg/src/main.rs

@@ -184,8 +184,21 @@ This file should be transitively included in `main.html`
   config.included_files = collect_included_files(&config, processor.as_ref())
     .expect("Failed to collect include files");
 
-  let markdown_files = collect_markdown_files(&input_dir);
-  generate_search_index(&config, &markdown_files)
+  let all_markdown_files = collect_markdown_files(&input_dir);
+
+  // Filter out included files - only standalone files should be in search index
+  let searchable_files: Vec<_> = all_markdown_files
+    .iter()
+    .filter(|file| {
+      file
+        .strip_prefix(&input_dir)
+        .ok()
+        .map_or(true, |rel| !config.included_files.contains_key(rel))
+    })
+    .cloned()
+    .collect();
+
+  generate_search_index(&config, &searchable_files)
     .expect("Failed to generate search index");
 
   // Verify that search data is generated correctly
@@ -194,27 +207,179 @@ This file should be transitively included in `main.html`
       .expect("Failed to open search-data.json");
   let search_data: Vec<SearchDocument> =
     serde_json::from_reader(index_file).expect("Failed to read index data");
-  let included_doc = search_data
+
+  // Only the main file should appear in search index
+  // The included files' content is already in main.html, so they're searchable
+  // through the main document
+  let main_doc = search_data
     .iter()
-    .find(|doc| doc.title == "Included file")
-    .expect("included file not found in search-data.json");
+    .find(|doc| doc.title == "Main file")
+    .expect("main file not found in search-data.json");
+
+  assert_eq!(main_doc.path, "main.html");
+
+  // Included files should NOT have separate search entries
+  assert!(
+    search_data.iter().all(|doc| doc.title != "Included file"),
+    "Included files should not have separate search entries"
+  );
+  assert!(
+    search_data
+      .iter()
+      .all(|doc| doc.title != "Section without an anchor ID"),
+    "Included files should not have separate search entries"
+  );
+  assert!(
+    search_data
+      .iter()
+      .all(|doc| doc.title != "Transitively included file"),
+    "Included files should not have separate search entries"
+  );
+}
+
+#[test]
+fn test_nested_directory_include_search_paths() {
+  // This test replicates a real-world scenario where:
+  // - index.md includes installation/modules.md
+  // - installation/modules.md includes installation/modules/nixos.md
+  // The search index should NOT create entries for included files as
+  // standalone pages (e.g., installation/modules/nixos.html), but should
+  // index their content under the root document with anchors.
+
+  let temp_dir = TempDir::new().expect("Failed to create temp dir");
+  let input_dir = temp_dir.path().join("input");
+  let installation_dir = input_dir.join("installation");
+  let modules_dir = installation_dir.join("modules");
+  let output_dir = temp_dir.path().join("output");
+
+  create_dir_all(&modules_dir).expect("failed to create modules dir");
+  create_dir_all(&output_dir).expect("failed to create output dir");
+
+  // Root document that includes a file from a subdirectory
+  let index_content = "# Documentation Index
+
+Welcome to the documentation.
+
+```{=include=}
+installation/modules.md
+```
+";
+  fs::write(input_dir.join("index.md"), index_content)
+    .expect("Failed to write index.md");
+
+  // Intermediate file that includes deeper nested files
+  let modules_content = "# Module Installation {#ch-module-installation}
+
+The below chapters describe module installation.
+
+```{=include=}
+modules/nixos.md
+modules/home-manager.md
+```
+";
+  fs::write(installation_dir.join("modules.md"), modules_content)
+    .expect("Failed to write installation/modules.md");
+
+  // Deeply nested included files
+  let nixos_content = "# NixOS Module {#ch-nixos-module}
+
+This describes the NixOS module installation.
+";
+  fs::write(modules_dir.join("nixos.md"), nixos_content)
+    .expect("Failed to write installation/modules/nixos.md");
+
+  let hm_content = "# Home Manager Module {#ch-home-manager-module}
 
-  assert_eq!(included_doc.path, "main.html#included-file-heading");
+This describes the Home Manager module installation.
+";
+  fs::write(modules_dir.join("home-manager.md"), hm_content)
+    .expect("Failed to write installation/modules/home-manager.md");
+
+  let mut config = Config {
+    input_dir: Some(input_dir.clone()),
+    output_dir: output_dir.clone(),
+    module_options: None,
+    title: "Test Documentation".to_string(),
+    generate_search: true,
+    ..Default::default()
+  };
 
-  let no_id_doc = search_data
+  let processor = Some(create_processor(&config, None));
+  config.included_files = collect_included_files(&config, processor.as_ref())
+    .expect("Failed to collect include files");
+
+  let all_markdown_files = collect_markdown_files(&input_dir);
+
+  // Process markdown files to generate HTML
+  ndg::utils::process_markdown_files(&config, processor.as_ref())
+    .expect("Failed to process markdown files");
+
+  // Filter out included files - only standalone files should be in search index
+  let searchable_files: Vec<_> = all_markdown_files
     .iter()
-    .find(|doc| doc.title == "Section without an anchor ID")
-    .expect("section_no_id file not found in search-data.json");
+    .filter(|file| {
+      file
+        .strip_prefix(&input_dir)
+        .ok()
+        .map_or(true, |rel| !config.included_files.contains_key(rel))
+    })
+    .cloned()
+    .collect();
+
+  // Generate search index with only standalone files
+  generate_search_index(&config, &searchable_files)
+    .expect("Failed to generate search index");
 
-  assert_eq!(no_id_doc.path, "main.html#section-without-an-anchor-id");
+  // Verify that search data is generated correctly
+  let index_file =
+    File::open(output_dir.join("assets").join("search-data.json"))
+      .expect("Failed to open search-data.json");
+  let search_data: Vec<SearchDocument> =
+    serde_json::from_reader(index_file).expect("Failed to read index data");
 
-  let transitive_inc_doc = search_data
+  // The index document should be in search results
+  let index_doc = search_data
     .iter()
-    .find(|doc| doc.title == "Transitively included file")
-    .expect("transitively included file not found in search-data.json");
+    .find(|doc| doc.title == "Documentation Index");
+  assert!(
+    index_doc.is_some(),
+    "Index document should be in search results"
+  );
+  assert_eq!(index_doc.unwrap().path, "index.html");
+
+  // Included files should NOT appear as separate search entries
+  // Their content is already in index.html
+  assert!(
+    search_data
+      .iter()
+      .all(|doc| doc.title != "Module Installation"),
+    "Included files should not have separate search entries"
+  );
+  assert!(
+    search_data.iter().all(|doc| doc.title != "NixOS Module"),
+    "Included files should not have separate search entries"
+  );
+  assert!(
+    search_data
+      .iter()
+      .all(|doc| doc.title != "Home Manager Module"),
+    "Included files should not have separate search entries"
+  );
 
-  assert_eq!(
-    transitive_inc_doc.path,
-    "main.html#transitively-included-file"
+  // Verify that the included files are NOT created as standalone HTML files
+  assert!(
+    !output_dir.join("installation/modules.html").exists(),
+    "installation/modules.html should not be created (file is included)"
+  );
+  assert!(
+    !output_dir.join("installation/modules/nixos.html").exists(),
+    "installation/modules/nixos.html should not be created (file is included)"
+  );
+  assert!(
+    !output_dir
+      .join("installation/modules/home-manager.html")
+      .exists(),
+    "installation/modules/home-manager.html should not be created (file is \
+     included)"
   );
 }