chore(redirects): fixup redirects across all locales (#46)

* add fix redirects command

* Add fixing redirects to sync translated content

* better help strings on CLI, comment on fix_redirect function

---------

Co-authored-by: Florian Dieminger <[email protected]>

Author: argl

crates/rari-cli/main.rs

@@ -25,6 +25,7 @@ use rari_tools::add_redirect::add_redirect;
 use rari_tools::history::gather_history;
 use rari_tools::popularities::update_popularities;
 use rari_tools::r#move::r#move;
+use rari_tools::redirects::fix_redirects;
 use rari_tools::remove::remove;
 use rari_tools::sidebars::fmt_sidebars;
 use rari_tools::sync_translated_content::sync_translated_content;
@@ -65,6 +66,7 @@ enum Commands {
     Popularities,
     Update(UpdateArgs),
     ExportSchema(ExportSchemaArgs),
+    /// Subcommands for altering content programmatically
     #[command(subcommand)]
     Content(ContentSubcommand),
 }
@@ -76,12 +78,23 @@ struct ExportSchemaArgs {
 
 #[derive(Subcommand)]
 enum ContentSubcommand {
-    /// Moves content from one slug to another
+    /// Moves content pages from one slug to another.
     Move(MoveArgs),
+    /// Deletes content pages.
     Delete(DeleteArgs),
+    /// Adds a redirect from->to pair to the redirect map.
+    ///
+    /// The locale is inferred from the from_url.
     AddRedirect(AddRedirectArgs),
+    /// Syncs translated content for all or a list of locales.
     SyncTranslatedContent(SyncTranslatedContentArgs),
+    /// Formats all sidebars.
     FmtSidebars,
+    /// Fixes redirects across all locales.
+    ///
+    /// This shortens multiple redirect chains to single ones.
+    /// This is also run as part of sync_translated_content.
+    FixRedirects,
 }
 
 #[derive(Args)]
@@ -387,6 +400,9 @@ fn main() -> Result<(), Error> {
             ContentSubcommand::FmtSidebars => {
                 fmt_sidebars()?;
             }
+            ContentSubcommand::FixRedirects => {
+                fix_redirects()?;
+            }
         },
         Commands::Update(args) => update(args.version)?,
         Commands::ExportSchema(args) => export_schema(args)?,

crates/rari-tools/src/redirects.rs

@@ -19,9 +19,7 @@ use url::Url;
 use crate::error::{RedirectError, ToolError};
 
 const REDIRECT_FILE_HEADER: &str = r#"# DO NOT EDIT THIS FILE MANUALLY.
-# Use the CLI instead::
-#
-#    rari content add-redirect <fromURL> <toURL>
+# Use the CLI instead.
 #
 # FROM-URL	TO-URL
 "#;
@@ -242,6 +240,57 @@ pub fn add_redirects(locale: Locale, update_pairs: &[(String, String)]) -> Resul
     Ok(())
 }
 
+/// Optimizes redirect rules based on all suported locales.
+///
+/// It loads all redirects into a singel structure and runs the short_cuts algorithm on it.
+/// The goal is to optimize redirects across locale boundaries.
+///
+/// This function overwrites all redirect files.
+///
+/// # Returns
+///
+/// - `Ok(())` if all operations complete successfully.
+/// - `Err(ToolError)` if any step fails, such as:
+///     - Invalid locale strings that cannot be parsed.
+///     - I/O errors during file reading or writing.
+///     - Errors from the `short_cuts` processing function.
+pub fn fix_redirects() -> Result<(), ToolError> {
+    let locales = Locale::for_generic_and_spas();
+    let mut pairs = HashMap::new();
+    for locale in locales {
+        let path = redirects_path(*locale)?;
+        read_redirects_raw(&path, &mut pairs)?;
+    }
+    let clean_pairs: HashMap<String, String> = short_cuts(&pairs)?.into_iter().collect();
+
+    // Split the clean pairs into their respective locales
+    let locale_pairs = clean_pairs.into_iter().try_fold(
+        HashMap::<Locale, HashMap<String, String>>::new(),
+        |mut acc, (from, to)| -> Result<HashMap<Locale, HashMap<String, String>>, ToolError> {
+            // Extract the locale string from the 'from' path
+            let locale_str = from.split('/').nth(1).unwrap_or_default();
+
+            // Parse the locale string into a Locale enum
+            let locale = Locale::from_str(locale_str).map_err(|_| {
+                ToolError::InvalidLocale(Cow::Owned(format!(
+                    "Cannot detect a valid locale for '{}'. Search for a '{}\t{}' pair in all _redirects.txt files to locate the problem.",
+                    from, from, to
+                )))
+            })?;
+            let locale_map = acc.entry(locale).or_default();
+            locale_map.insert(from, to);
+            Ok(acc)
+        },
+    )?;
+
+    // Write the updated maps back to their redirects files
+    for (locale, pairs) in &locale_pairs {
+        let path = redirects_path(*locale)?;
+        write_redirects(&path, pairs)?;
+    }
+    Ok(())
+}
+
 /// Gets the path to the redirects file for a specific locale.
 pub(crate) fn redirects_path(locale: Locale) -> Result<PathBuf, ToolError> {
     let root = root_for_locale(locale)?;

crates/rari-tools/src/sync_translated_content.rs

@@ -13,7 +13,7 @@ use sha2::{Digest, Sha256};
 
 use crate::error::ToolError;
 use crate::git::exec_git_with_test_fallback;
-use crate::redirects::add_redirects;
+use crate::redirects::{add_redirects, fix_redirects};
 use crate::utils::{get_redirects_map, read_all_doc_pages};
 use crate::wikihistory::update_wiki_history;
 
@@ -31,11 +31,16 @@ pub fn sync_translated_content(
         println!(
             "{}",
             green.apply_to(format!(
-                "Syncing translated content for locales: {:?}. Reading documents.",
+                "Syncing translated content for locales: {:?}.\nFixing cross-locale redirects.",
                 locales
             )),
         );
     }
+    fix_redirects()?;
+
+    if verbose {
+        println!("{}", green.apply_to("Reading all documents."));
+    }
 
     let docs = read_all_doc_pages()?;
     let redirects_maps: HashMap<Locale, HashMap<String, String>> = locales