chore(cli): add sidebar support to content move/delete commands (#41)

* intermediate commit * first cut working * handle path fields as well * use a btreemap for sidebar l10n, some comments, add a warning line about losing comments into the sidebar files * use IndexMap on sidebar translations * some re-shuffling and a happy path test * comment update * moved some crate deps to workspace level, format sidebar yaml with pretty_yaml, do not change sidebars on translated content, implement clone and partial_eq on sidebar structs, only write sidebars if they changed * path fields have absolute urls into the locale default * add hash support to sidebar tool * intermediate commit * support removing items from sidebars by introducing a `None` entry type that gets filtered out on serializing. more happy tests, fixed missing hash serialization skip test, changed update_sidebars function signature to allow option<string> for the `to` parameters, factored out reading the sidebars. * clean up * more clean up * add fmt-sidebars * fix one case --------- Co-authored-by: Florian Dieminger <[email protected]>
mdn · Nov 18, 2024 · 6c10c8d · 6c10c8d
1 parent 5dfe87a
commit 6c10c8d
Show file tree

Hide file tree

Showing 18 changed files with 657 additions and 44 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -91,7 +91,10 @@ base64 = "0.22"
 console = "0.15"
 sha2 = "0.10"
 dashmap = "6"
-
+serde_yaml_ng = "0.10"
+pretty_yaml = "0.5"
+yaml_parser = "0.2"
+const_format = "0.2"
 
 [dependencies]
 rari-doc.workspace = true

diff --git a/crates/diff-test/src/main.rs b/crates/diff-test/src/main.rs
@@ -377,9 +377,7 @@ static ALLOWLIST: LazyLock<HashSet<(&str, &str)>> = LazyLock::new(|| {
         // image dimension rounding error or similar, ok
         ("docs/web/media/formats/video_concepts/index.json", "doc.body.3.value.content"),
         ("docs/web/svg/tutorial/introduction/index.json", "doc.body.0.value.content"),
-        ("docs/web/css/css_flexible_box_layout/basic_concepts_of_flexbox/index.json", "doc.body.2.value.content"),
-        ("docs/web/css/css_flexible_box_layout/basic_concepts_of_flexbox/index.json", "doc.body.3.value.content"),
-        // rari improvement over yari
+        // rari macro improvement
         ("docs/web/manifest/index.json", "doc.body.1.value.content"),
         ("docs/web/manifest/orientation/index.json", "doc.body.6.value.content"),
         ("docs/web/css/-moz-orient/index.json", "doc.body.3.value.content"),

diff --git a/crates/rari-cli/main.rs b/crates/rari-cli/main.rs
@@ -24,6 +24,7 @@ use rari_tools::history::gather_history;
 use rari_tools::popularities::update_popularities;
 use rari_tools::r#move::r#move;
 use rari_tools::remove::remove;
+use rari_tools::sidebars::fmt_sidebars;
 use rari_tools::sync_translated_content::sync_translated_content;
 use rari_types::globals::{build_out_root, content_root, content_translated_root, SETTINGS};
 use rari_types::locale::Locale;
@@ -71,6 +72,7 @@ enum ContentSubcommand {
     Delete(DeleteArgs),
     AddRedirect(AddRedirectArgs),
     SyncTranslatedContent(SyncTranslatedContentArgs),
+    FmtSidebars,
 }
 
 #[derive(Args)]
@@ -379,6 +381,9 @@ fn main() -> Result<(), Error> {
                 let locales = args.locales.as_deref().unwrap_or(Locale::translated());
                 sync_translated_content(locales, cli.verbose.is_present())?;
             }
+            ContentSubcommand::FmtSidebars => {
+                fmt_sidebars()?;
+            }
         },
         Commands::Update(args) => update(args.version)?,
     }

diff --git a/crates/rari-doc/Cargo.toml b/crates/rari-doc/Cargo.toml
@@ -25,11 +25,11 @@ indexmap.workspace = true
 sha2.workspace = true
 tracing-subscriber.workspace = true
 dashmap.workspace = true
+pretty_yaml.workspace = true
+serde_yaml_ng.workspace = true
+yaml_parser.workspace = true
 
-serde_yaml_ng = "0.10"
 yaml-rust = "0.4"
-pretty_yaml = "0.5"
-yaml_parser = "0.2"
 percent-encoding = "2"
 pest = "2"
 pest_derive = "2"

diff --git a/crates/rari-doc/src/cached_readers.rs b/crates/rari-doc/src/cached_readers.rs
@@ -149,7 +149,7 @@ pub fn read_sidebar(name: &str, locale: Locale, slug: &str) -> Result<Arc<MetaSi
             file.set_extension("yaml");
             let raw = read_to_string(&file)?;
             let sidebar: Sidebar = serde_yaml_ng::from_str(&raw)?;
-            let sidebar = Arc::new(MetaSidebar::from(sidebar));
+            let sidebar = Arc::new(MetaSidebar::try_from(sidebar)?);
             if cache_content() {
                 CACHED_SIDEBAR_FILES.insert(key, sidebar.clone());
             }

diff --git a/crates/rari-doc/src/error.rs b/crates/rari-doc/src/error.rs
@@ -102,6 +102,8 @@ pub enum DocError {
     RariIoError(#[from] rari_utils::error::RariIoError),
     #[error("Slug required for SidebarEntry")]
     SlugRequiredForSidebarEntry,
+    #[error("Invalid sidebar entry")]
+    InvalidSidebarEntry,
 }
 
 /// Represents various errors that can occur while processing URLs.

diff --git a/crates/rari-doc/src/html/sidebar.rs b/crates/rari-doc/src/html/sidebar.rs
@@ -3,12 +3,13 @@ pub use std::ops::Deref;
 use std::sync::{Arc, LazyLock};
 
 use dashmap::DashMap;
+use indexmap::IndexMap;
 use rari_types::fm_types::PageType;
 use rari_types::globals::cache_content;
 use rari_types::locale::Locale;
 use rari_utils::concat_strs;
 use scraper::{Html, Node, Selector};
-use serde::{Deserialize, Serialize};
+use serde::{Deserialize, Serialize, Serializer};
 
 use super::links::{render_link_from_page, render_link_via_page, LinkModifier};
 use super::modifier::insert_attribute;
@@ -19,7 +20,7 @@ use crate::helpers;
 use crate::helpers::subpages::{list_sub_pages_grouped_internal, list_sub_pages_internal};
 use crate::pages::page::{Page, PageLike};
 use crate::pages::types::doc::Doc;
-use crate::utils::t_or_vec;
+use crate::utils::{is_default, serialize_t_or_vec, t_or_vec};
 
 fn cache_side_bar(sidebar: &str) -> bool {
     cache_content()
@@ -136,10 +137,12 @@ pub fn build_sidebars(doc: &Doc) -> Result<Option<String>, DocError> {
     Ok(if out.is_empty() { None } else { Some(out) })
 }
 
-#[derive(Serialize, Deserialize, Default, Debug)]
+#[derive(Serialize, Deserialize, Default, Debug, PartialEq, Clone)]
 #[serde(transparent)]
 pub struct SidebarL10n {
-    l10n: HashMap<Locale, HashMap<String, String>>,
+    // Keep the translations in order of insertion,
+    // so Sidebar manipulations are deterministic.
+    l10n: IndexMap<Locale, IndexMap<String, String>>,
 }
 
 impl SidebarL10n {
@@ -149,15 +152,36 @@ impl SidebarL10n {
             return s.map(|s| s.as_str()).unwrap_or(key);
         }
         self.l10n
-            .get(&Default::default())
+            .get(&Locale::default())
             .and_then(|l| l.get(key))
             .map(|s| s.as_str())
             .unwrap_or(key)
     }
 }
 
-#[derive(Serialize, Deserialize, Default, Debug)]
+// Serialize the sidebar entries, filtering out the None variant. This is
+// used on the top-level sidebar field and the basic entry children field.
+fn serialize_sidebar_entries<S>(sidebar: &[SidebarEntry], serializer: S) -> Result<S::Ok, S::Error>
+where
+    S: Serializer,
+{
+    // Filter out the None variant
+    let filtered: Vec<&SidebarEntry> = sidebar
+        .iter()
+        .filter(|entry| !matches!(entry, SidebarEntry::None))
+        .collect();
+    filtered.serialize(serializer)
+}
+
+fn sidebar_entries_are_empty(entries: &[SidebarEntry]) -> bool {
+    entries
+        .iter()
+        .all(|entry| matches!(entry, SidebarEntry::None))
+}
+
+#[derive(Serialize, Deserialize, Default, Debug, PartialEq, Clone)]
 pub struct Sidebar {
+    #[serde(serialize_with = "serialize_sidebar_entries")]
     pub sidebar: Vec<SidebarEntry>,
     #[serde(default)]
     pub l10n: SidebarL10n,
@@ -168,12 +192,18 @@ pub struct MetaSidebar {
     pub entries: Vec<SidebarMetaEntry>,
     pub l10n: SidebarL10n,
 }
-impl From<Sidebar> for MetaSidebar {
-    fn from(value: Sidebar) -> Self {
-        MetaSidebar {
-            entries: value.sidebar.into_iter().map(Into::into).collect(),
+impl TryFrom<Sidebar> for MetaSidebar {
+    type Error = DocError;
+
+    fn try_from(value: Sidebar) -> Result<Self, Self::Error> {
+        Ok(MetaSidebar {
+            entries: value
+                .sidebar
+                .into_iter()
+                .map(TryInto::try_into)
+                .collect::<Result<_, DocError>>()?,
             l10n: value.l10n,
-        }
+        })
     }
 }
 
@@ -198,42 +228,62 @@ impl MetaSidebar {
     }
 }
 
-#[derive(Serialize, Deserialize, Default, Debug)]
-#[serde(rename_all = "camelCase", tag = "type")]
+// used for skipping serialization if the field has the defaul value
+fn details_is_none(details: &Details) -> bool {
+    matches!(details, Details::None)
+}
+
+#[derive(Serialize, Deserialize, Default, Debug, PartialEq, Clone)]
+#[serde(rename_all = "camelCase")]
 pub struct BasicEntry {
-    pub title: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub link: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub hash: Option<String>,
-    #[serde(default)]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub title: Option<String>,
+    #[serde(default, skip_serializing_if = "details_is_none")]
+    pub details: Details,
+    #[serde(default, skip_serializing_if = "is_default")]
     pub code: bool,
-    #[serde(default)]
+    #[serde(
+        default,
+        skip_serializing_if = "sidebar_entries_are_empty",
+        serialize_with = "serialize_sidebar_entries"
+    )]
     pub children: Vec<SidebarEntry>,
-    #[serde(default)]
-    pub details: Details,
 }
 
-#[derive(Serialize, Deserialize, Default, Debug)]
-#[serde(rename_all = "camelCase", tag = "type")]
+#[derive(Serialize, Deserialize, Default, Debug, PartialEq, Clone)]
+#[serde(rename_all = "camelCase")]
 pub struct SubPageEntry {
     pub path: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub title: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub link: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
     pub hash: Option<String>,
-    #[serde(deserialize_with = "t_or_vec", default)]
+    #[serde(
+        default,
+        deserialize_with = "t_or_vec",
+        serialize_with = "serialize_t_or_vec",
+        skip_serializing_if = "Vec::is_empty"
+    )]
     pub tags: Vec<PageType>,
-    #[serde(default)]
+    #[serde(default, skip_serializing_if = "details_is_none")]
     pub details: Details,
-    #[serde(default)]
+    #[serde(default, skip_serializing_if = "is_default")]
     pub include_parent: bool,
 }
 
-#[derive(Serialize, Deserialize, Default, Debug)]
-#[serde(rename_all = "camelCase", tag = "type")]
+#[derive(Serialize, Deserialize, Default, Debug, PartialEq, Clone)]
+#[serde(rename_all = "camelCase")]
 pub struct WebExtApiEntry {
     pub title: String,
 }
 
-#[derive(Serialize, Deserialize, Debug)]
+#[derive(Serialize, Deserialize, Debug, PartialEq, Clone)]
 #[serde(rename_all = "camelCase", tag = "type")]
 pub enum SidebarEntry {
     Section(BasicEntry),
@@ -244,6 +294,8 @@ pub enum SidebarEntry {
     Default(BasicEntry),
     #[serde(untagged)]
     Link(String),
+    #[serde(untagged)]
+    None,
 }
 
 #[derive(Debug, Default)]
@@ -320,9 +372,10 @@ pub struct SidebarMetaEntry {
     pub children: MetaChildren,
 }
 
-impl From<SidebarEntry> for SidebarMetaEntry {
-    fn from(value: SidebarEntry) -> Self {
-        match value {
+impl TryFrom<SidebarEntry> for SidebarMetaEntry {
+    type Error = DocError;
+    fn try_from(value: SidebarEntry) -> Result<Self, Self::Error> {
+        let res = match value {
             SidebarEntry::Section(BasicEntry {
                 link,
                 hash,
@@ -338,7 +391,12 @@ impl From<SidebarEntry> for SidebarMetaEntry {
                 children: if children.is_empty() {
                     MetaChildren::None
                 } else {
-                    MetaChildren::Children(children.into_iter().map(Into::into).collect())
+                    MetaChildren::Children(
+                        children
+                            .into_iter()
+                            .map(TryInto::try_into)
+                            .collect::<Result<_, DocError>>()?,
+                    )
                 },
             },
             SidebarEntry::ListSubPages(SubPageEntry {
@@ -386,7 +444,12 @@ impl From<SidebarEntry> for SidebarMetaEntry {
                 children: if children.is_empty() {
                     MetaChildren::None
                 } else {
-                    MetaChildren::Children(children.into_iter().map(Into::into).collect())
+                    MetaChildren::Children(
+                        children
+                            .into_iter()
+                            .map(TryInto::try_into)
+                            .collect::<Result<_, DocError>>()?,
+                    )
                 },
             },
             SidebarEntry::Link(link) => SidebarMetaEntry {
@@ -403,7 +466,9 @@ impl From<SidebarEntry> for SidebarMetaEntry {
                 content: SidebarMetaEntryContent::from_link_title_hash(None, Some(title), None),
                 children: MetaChildren::WebExtApi,
             },
-        }
+            SidebarEntry::None => return Err(DocError::InvalidSidebarEntry),
+        };
+        Ok(res)
     }
 }
 

diff --git a/crates/rari-doc/src/utils.rs b/crates/rari-doc/src/utils.rs
@@ -384,3 +384,7 @@ pub fn trim_fefore<'a>(input: &'a str, pat: Option<&str>) -> &'a str {
     }
     input
 }
+
+pub fn is_default<T: PartialEq + Default>(value: &T) -> bool {
+    value == &T::default()
+}
-Original file line number
+Diff line change
@@ Expand Up @@
         }
         input
     }
+    pub fn is_default<T: PartialEq + Default>(value: &T) -> bool {
+        value == &T::default()
+    }