Merge pull request google#100 from google/summary-formatting

Allow formatting in the `SUMMARY.md` file

Author: mgeisler

i18n-helpers/src/bin/mdbook-gettext.rs

@@ -30,17 +30,67 @@ use mdbook::preprocess::{CmdPreprocessor, PreprocessorContext};
 use mdbook::BookItem;
 use mdbook_i18n_helpers::{extract_events, reconstruct_markdown, translate_events};
 use polib::catalog::Catalog;
+use polib::message::Message;
 use polib::po_file;
+use pulldown_cmark::Event;
 use semver::{Version, VersionReq};
 use std::{io, process};
 
+/// Strip formatting from a Markdown string.
+///
+/// The string can only contain inline text. Formatting such as
+/// emphasis and strong emphasis is removed.
+///
+/// Modelled after `mdbook::summary::stringify_events`.
+fn strip_formatting(text: &str) -> String {
+    extract_events(text, None)
+        .iter()
+        .filter_map(|(_, event)| match event {
+            Event::Text(text) | Event::Code(text) => Some(text.as_ref()),
+            Event::SoftBreak => Some(" "),
+            _ => None,
+        })
+        .collect()
+}
+
 fn translate(text: &str, catalog: &Catalog) -> String {
     let events = extract_events(text, None);
     let translated_events = translate_events(&events, catalog);
     let (translated, _) = reconstruct_markdown(&translated_events, None);
     translated
 }
 
+/// Update `catalog` with stripped messages from `SUMMARY.md`.
+///
+/// While it is permissible to include formatting in the `SUMMARY.md`
+/// file, `mdbook` will strip it out when rendering the book. It will
+/// also strip formatting when sending the book to preprocessors.
+///
+/// To be able to find the translations for the `SUMMARY.md` file, we
+/// append versions of these messages stripped of formatting.
+fn add_stripped_summary_translations(catalog: &mut Catalog) {
+    let mut stripped_messages = Vec::new();
+    for msg in catalog.messages() {
+        // The `SUMMARY.md` filename is fixed, but we cannot assume
+        // that the file is at `src/SUMMARY.md` since the `src/`
+        // directory can be configured.
+        if !msg.source().contains("SUMMARY.md") {
+            continue;
+        }
+
+        let message = Message::build_singular()
+            .with_msgid(strip_formatting(msg.msgid()))
+            .with_msgstr(strip_formatting(msg.msgstr().unwrap()))
+            .done();
+        stripped_messages.push(message);
+    }
+
+    for msg in stripped_messages {
+        catalog.append_or_update(msg);
+    }
+}
+
+/// Translte an entire book.
 fn translate_book(ctx: &PreprocessorContext, mut book: Book) -> anyhow::Result<Book> {
     // Translation is a no-op when the target language is not set
     let language = match &ctx.config.book.language {
@@ -60,9 +110,10 @@ fn translate_book(ctx: &PreprocessorContext, mut book: Book) -> anyhow::Result<B
         return Ok(book);
     }
 
-    let catalog = po_file::parse(&path)
+    let mut catalog = po_file::parse(&path)
         .map_err(|err| anyhow!("{err}"))
         .with_context(|| format!("Could not parse {:?} as PO file", path))?;
+    add_stripped_summary_translations(&mut catalog);
     book.for_each_mut(|item| match item {
         BookItem::Chapter(ch) => {
             ch.content = translate(&ch.content, &catalog);
@@ -114,7 +165,7 @@ fn main() -> anyhow::Result<()> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use polib::message::Message;
+    use polib::message::{Message, MessageMutView};
     use polib::metadata::CatalogMetadata;
     use pretty_assertions::assert_eq;
 
@@ -130,6 +181,37 @@ mod tests {
         catalog
     }
 
+    #[test]
+    fn test_add_stripped_summary_translations() {
+        // Add two messages which map to the same stripped message.
+        let mut catalog = create_catalog(&[
+            ("foo `bar`", "FOO `BAR`"),
+            ("**foo** _bar_", "**FOO** _BAR_"),
+        ]);
+        for (idx, mut msg) in catalog.messages_mut().enumerate() {
+            // Set the source to SUMMARY.md to ensure
+            // add_stripped_summary_translations will add a stripped
+            // version.
+            *msg.source_mut() = format!("src/SUMMARY.md:{idx}");
+        }
+        add_stripped_summary_translations(&mut catalog);
+
+        // We now have two messages, one with and one without
+        // formatting. This lets us handle both the TOC and any
+        // occurance on the page.
+        assert_eq!(
+            catalog
+                .messages()
+                .map(|msg| (msg.source(), msg.msgid(), msg.msgstr().unwrap()))
+                .collect::<Vec<_>>(),
+            &[
+                ("src/SUMMARY.md:0", "foo `bar`", "FOO `BAR`"),
+                ("src/SUMMARY.md:1", "**foo** _bar_", "**FOO** _BAR_"),
+                ("", "foo bar", "FOO BAR")
+            ]
+        );
+    }
+
     #[test]
     fn test_translate_single_line() {
         let catalog = create_catalog(&[("foo bar", "FOO BAR")]);

i18n-helpers/src/bin/mdbook-xgettext.rs

@@ -22,12 +22,27 @@
 use anyhow::{anyhow, Context};
 use mdbook::renderer::RenderContext;
 use mdbook::BookItem;
-use mdbook_i18n_helpers::extract_messages;
+use mdbook_i18n_helpers::{extract_events, extract_messages, reconstruct_markdown};
 use polib::catalog::Catalog;
 use polib::message::Message;
 use polib::metadata::CatalogMetadata;
+use pulldown_cmark::{Event, Tag};
 use std::{fs, io};
 
+/// Strip an optional link from a Markdown string.
+fn strip_link(text: &str) -> String {
+    let events = extract_events(text, None)
+        .into_iter()
+        .filter_map(|(_, event)| match event {
+            Event::Start(Tag::Link(..)) => None,
+            Event::End(Tag::Link(..)) => None,
+            _ => Some((0, event)),
+        })
+        .collect::<Vec<_>>();
+    let (without_link, _) = reconstruct_markdown(&events, None);
+    without_link
+}
+
 fn add_message(catalog: &mut Catalog, msgid: &str, source: &str) {
     let wrap_options = textwrap::Options::new(76)
         .break_words(false)
@@ -59,31 +74,17 @@ fn create_catalog(ctx: &RenderContext) -> anyhow::Result<Catalog> {
     let mut catalog = Catalog::new(metadata);
 
     // First, add all chapter names and part titles from SUMMARY.md.
-    // The book items are in order of the summary, so we can assign
-    // correct line numbers for duplicate lines by tracking the index
-    // of our last search.
     let summary_path = ctx.config.book.src.join("SUMMARY.md");
     let summary = std::fs::read_to_string(ctx.root.join(&summary_path))
         .with_context(|| anyhow!("Failed to read {}", summary_path.display()))?;
-    let mut last_idx = 0;
-    for item in ctx.book.iter() {
-        let line = match item {
-            BookItem::Chapter(chapter) => &chapter.name,
-            BookItem::PartTitle(title) => title,
-            BookItem::Separator => continue,
-        };
-
-        let idx = summary[last_idx..].find(line).ok_or_else(|| {
-            anyhow!(
-                "Could not find {line:?} in SUMMARY.md after line {} -- \
-                 please remove any formatting from SUMMARY.md",
-                summary[..last_idx].lines().count()
-            )
-        })?;
-        last_idx += idx;
-        let lineno = summary[..last_idx].lines().count();
+    for (lineno, msgid) in extract_messages(&summary) {
         let source = format!("{}:{}", summary_path.display(), lineno);
-        add_message(&mut catalog, line, &source);
+        // The summary is mostly links like "[Foo *Bar*](foo-bar.md)".
+        // We strip away the link to get "Foo *Bar*". The formatting
+        // is stripped away by mdbook when it sends the book to
+        // mdbook-gettext -- we keep the formatting here in case the
+        // same text is used for the page title.
+        add_message(&mut catalog, &strip_link(&msgid), &source);
     }
 
     // Next, we add the chapter contents.
@@ -147,6 +148,22 @@ mod tests {
         Ok((ctx, tmpdir))
     }
 
+    #[test]
+    fn test_strip_link_empty() {
+        assert_eq!(strip_link(""), "");
+    }
+
+    #[test]
+    fn test_strip_link_text() {
+        assert_eq!(strip_link("Summary"), "Summary");
+    }
+
+    #[test]
+    fn test_strip_link_with_formatting() {
+        // The formatting is automatically normalized.
+        assert_eq!(strip_link("[foo *bar* `baz`](foo.md)"), "foo _bar_ `baz`");
+    }
+
     #[test]
     fn test_create_catalog_defaults() -> anyhow::Result<()> {
         let (ctx, _tmp) =
@@ -183,29 +200,63 @@ mod tests {
 
     #[test]
     fn test_create_catalog_summary_formatting() -> anyhow::Result<()> {
-        // It is an error to include formatting in the summary file:
-        // it is stripped by mdbook and we cannot find it later when
-        // trying to translate the book.
         let (ctx, _tmp) = create_render_context(&[
             ("book.toml", "[book]"),
-            ("src/SUMMARY.md", "- [foo *bar* baz]()"),
+            (
+                "src/SUMMARY.md",
+                "# Summary\n\
+                 \n\
+                 [Prefix Chapter](prefix.md)\n\
+                 \n\
+                 # Part Title\n\
+                 \n\
+                 - [Foo *Bar*](foo.md)\n\
+                 \n\
+                 ----------\n\
+                 \n\
+                 - [Baz `Quux`](baz.md)\n\
+                 \n\
+                 [Suffix Chapter](suffix.md)",
+            ),
+            // Without this, mdbook would automatically create the
+            // files based on the summary above. This would add
+            // unnecessary headings below.
+            ("src/prefix.md", ""),
+            ("src/foo.md", ""),
+            ("src/baz.md", ""),
+            ("src/suffix.md", ""),
         ])?;
 
-        assert!(create_catalog(&ctx).is_err());
+        let catalog = create_catalog(&ctx)?;
+        assert_eq!(
+            catalog
+                .messages()
+                .map(|msg| msg.msgid())
+                .collect::<Vec<&str>>(),
+            &[
+                "Summary",
+                "Prefix Chapter",
+                "Part Title",
+                "Foo _Bar_",
+                "Baz `Quux`",
+                "Suffix Chapter",
+            ]
+        );
+
         Ok(())
     }
 
     #[test]
     fn test_create_catalog() -> anyhow::Result<()> {
         let (ctx, _tmp) = create_render_context(&[
             ("book.toml", "[book]"),
-            ("src/SUMMARY.md", "- [The Foo Chapter](foo.md)"),
+            ("src/SUMMARY.md", "- [The *Foo* Chapter](foo.md)"),
             (
                 "src/foo.md",
                 "# How to Foo\n\
                  \n\
-                 The first paragraph about Foo.\n\
-                 Still the first paragraph.\n",
+                 First paragraph.\n\
+                 Same paragraph.\n",
             ),
         ])?;
 
@@ -218,12 +269,12 @@ mod tests {
         assert_eq!(
             catalog
                 .messages()
-                .map(|msg| msg.msgid())
-                .collect::<Vec<&str>>(),
+                .map(|msg| (msg.source(), msg.msgid()))
+                .collect::<Vec<_>>(),
             &[
-                "The Foo Chapter",
-                "How to Foo",
-                "The first paragraph about Foo. Still the first paragraph.",
+                ("src/SUMMARY.md:1", "The _Foo_ Chapter"),
+                ("src/foo.md:1", "How to Foo"),
+                ("src/foo.md:3", "First paragraph. Same paragraph."),
             ]
         );