Merge pull request #326 from quarto-dev/feature/bd-5b21rbaq-video-shortcode-preview

fix(video): embed + size video shortcode in preview, hub-client, and revealjs render

Author: cscheid

claude-notes/plans/2026-06-22-video-shortcode-preview.md

@@ -138,6 +138,37 @@ pub fn is_revealjs_target(target_format: &str) -> bool {
     matches!(target_format, "revealjs" | "q2-slides")
 }
 
+/// The canonical Pandoc output format a Lua filter or shortcode should see as
+/// its `FORMAT` global, given the pipeline's `target_format`.
+///
+/// The preview pipeline runs with a *pseudo-format* `target_format`
+/// (`q2-preview`, `q2-slides`, …) so q2-core can branch on it for AST-vs-HTML
+/// output and reveal-tree construction (see [`builtin_pseudo_format`] /
+/// [`is_revealjs_target`]). But Lua extensions don't know about those
+/// pseudo-formats: their `quarto.doc.is_format("html:js")` /
+/// `is_format("revealjs")` checks (e.g. the built-in `video` shortcode) only
+/// recognize real Pandoc formats. If we hand the pseudo-format straight to Lua,
+/// those checks fail and format-gated shortcodes/filters silently degrade
+/// (`{{< video >}}` collapsed to a plain link in preview — bd-5b21rbaq).
+///
+/// This maps each preview pseudo-format back to the real format it emulates so
+/// preview Lua behaves identically to render:
+/// - `q2-preview` / `q2-debug` / `q2-raw` → `html`
+/// - `q2-slides` → `revealjs` (so `is_format("revealjs")` is true, matching
+///   what [`is_revealjs_target`] already does for pipeline decisions; note this
+///   intentionally differs from [`builtin_pseudo_format`], which reports the
+///   *output writer* base `html`).
+///
+/// Any non-pseudo `target_format` (`html`, `revealjs`, `latex`, …) passes
+/// through unchanged.
+pub fn lua_format_for(target_format: &str) -> &str {
+    match target_format {
+        "q2-preview" | "q2-debug" | "q2-raw" => "html",
+        "q2-slides" => "revealjs",
+        other => other,
+    }
+}
+
 /// Extract the chosen output format from a document's leading YAML
 /// front-matter: the `format:` scalar, or the first key when `format:` is a
 /// map (e.g. `format: {revealjs: {...}}`). Returns `None` when there is no
@@ -329,6 +360,35 @@ impl Format {
         }
     }
 
+    /// The canonical Pandoc output format a Lua filter or shortcode should see
+    /// as its `FORMAT` global.
+    ///
+    /// Lua extensions branch on real Pandoc formats
+    /// (`quarto.doc.is_format("html:js")`, `is_format("revealjs")`, …), not on
+    /// q2's preview pseudo-formats or extension-style format strings. This
+    /// resolves a `Format` to the base format it emulates:
+    /// - reveal targets (`revealjs` render **and** `q2-slides` preview) →
+    ///   `"revealjs"`, so `is_format("revealjs")` fires in preview too. The
+    ///   bare [`identifier`](Self::identifier) would otherwise collapse
+    ///   `q2-slides` to `Html` (its *output writer* is HTML), losing
+    ///   reveal-ness — the reason a user filter in a reveal preview saw
+    ///   `"html"` (bd-5b21rbaq).
+    /// - everything else → the identifier base (`html`, `pdf`, …), which
+    ///   already canonicalizes extension formats (`acm-pdf` → `pdf`) and the
+    ///   HTML preview pseudo-formats (`q2-preview`/`q2-debug`/`q2-raw` → `html`).
+    ///
+    /// This is the [`Format`]-aware companion to the string-only
+    /// [`lua_format_for`] (used where only a `target_format` string is in hand,
+    /// e.g. the transform-pipeline builder); the two agree on the pseudo-format
+    /// and base cases.
+    pub fn lua_format(&self) -> &str {
+        if is_revealjs_target(&self.target_format) {
+            "revealjs"
+        } else {
+            self.identifier.as_str()
+        }
+    }
+
     /// Parse a format string into a Format.
     ///
     /// Accepts:
@@ -821,6 +881,79 @@ mod tests {
         assert_eq!(f.pipeline_kind, Some("preview"));
     }
 
+    #[test]
+    fn test_lua_format_for_maps_preview_pseudo_formats() {
+        // HTML-emulating pseudo-formats resolve to `html` so
+        // `is_format("html:js")` fires in preview (bd-5b21rbaq).
+        assert_eq!(lua_format_for("q2-preview"), "html");
+        assert_eq!(lua_format_for("q2-debug"), "html");
+        assert_eq!(lua_format_for("q2-raw"), "html");
+        // The reveal preview pseudo-format resolves to `revealjs` so
+        // `is_format("revealjs")` fires — distinct from
+        // `builtin_pseudo_format`, which reports the output-writer base `html`.
+        assert_eq!(lua_format_for("q2-slides"), "revealjs");
+    }
+
+    #[test]
+    fn test_lua_format_for_passes_through_real_formats() {
+        for f in ["html", "revealjs", "latex", "pdf", "gfm", "typst", "docx"] {
+            assert_eq!(lua_format_for(f), f, "real format {f} must pass through");
+        }
+    }
+
+    #[test]
+    fn test_format_lua_format_canonicalizes() {
+        // Real formats: identifier base.
+        assert_eq!(
+            Format::from_format_string("html").unwrap().lua_format(),
+            "html"
+        );
+        assert_eq!(
+            Format::from_format_string("revealjs").unwrap().lua_format(),
+            "revealjs"
+        );
+        // Extension formats canonicalize to their base (the bug `identifier`
+        // already handled; `lua_format` must preserve it).
+        assert_eq!(
+            Format::from_format_string("acm-pdf").unwrap().lua_format(),
+            "pdf"
+        );
+        // HTML preview pseudo-formats → html.
+        assert_eq!(
+            Format::from_format_string("q2-preview")
+                .unwrap()
+                .lua_format(),
+            "html"
+        );
+        assert_eq!(
+            Format::from_format_string("q2-debug").unwrap().lua_format(),
+            "html"
+        );
+        // Reveal preview pseudo-format → revealjs (NOT its html output base) —
+        // the parity fix for user filters in reveal preview.
+        assert_eq!(
+            Format::from_format_string("q2-slides")
+                .unwrap()
+                .lua_format(),
+            "revealjs"
+        );
+    }
+
+    /// `Format::lua_format` and the string-only `lua_format_for` must agree on
+    /// the pseudo-format + base cases both handle, so the shortcode pipeline
+    /// (string-only) and user-filter stage (Format-aware) don't drift.
+    #[test]
+    fn test_lua_format_helpers_agree_on_shared_cases() {
+        for fmt in ["html", "revealjs", "q2-preview", "q2-slides", "q2-debug"] {
+            let f = Format::from_format_string(fmt).unwrap();
+            assert_eq!(
+                f.lua_format(),
+                lua_format_for(&f.target_format),
+                "lua_format helpers disagree for {fmt}"
+            );
+        }
+    }
+
     #[test]
     fn test_from_format_string_typst_extension() {
         let f = Format::from_format_string("typst").unwrap();

crates/quarto-core/src/format.rs

@@ -1113,14 +1113,22 @@ pub fn build_transform_pipeline(
     // True for `revealjs` (native render) and `q2-slides` (preview).
     let is_revealjs = crate::format::is_revealjs_target(&target_format);
 
+    // The Lua engines (shortcodes, user filters) see the *canonical* Pandoc
+    // format as their `FORMAT` global, not q2's preview pseudo-format. Under
+    // preview, `target_format` is `q2-preview` / `q2-slides`, which Lua's
+    // `is_format("html:js")` / `is_format("revealjs")` don't recognize — so
+    // format-gated shortcodes degrade (the `{{< video >}}` → plain-link bug,
+    // bd-5b21rbaq). Normalizing here makes preview Lua behave like render.
+    let lua_format = crate::format::lua_format_for(&target_format).to_string();
+
     // === NORMALIZATION PHASE ===
     pipeline.push(Box::new(CalloutTransform::new()));
     pipeline.push(Box::new(CalloutResolveTransform::new()));
     pipeline.push(Box::new(ShortcodeResolveTransform::with_lua_support(
         shortcode_paths,
         extensions,
         runtime,
-        target_format,
+        lua_format,
     )));
     pipeline.push(Box::new(MetadataNormalizeTransform::new()));
     // bd-1tl09 Phase 0: code-block decoration Generate runs after

crates/quarto-core/src/pipeline.rs

@@ -131,7 +131,12 @@ impl PipelineStage for UserFiltersStage {
                 .join(", ")
         );
 
-        let target_format = ctx.format.identifier.as_str();
+        // The canonical Pandoc format Lua filters see as FORMAT. Using
+        // `Format::lua_format()` (not `identifier.as_str()`) makes the reveal
+        // *preview* pseudo-format `q2-slides` resolve to `revealjs` rather than
+        // its HTML output-writer base — so a user filter's `is_format("revealjs")`
+        // fires in preview exactly as in native reveal render (bd-5b21rbaq).
+        let target_format = ctx.format.lua_format();
 
         // Build the attribution lookup handle when a sidecar is
         // present. `AttributionGenerateStage` runs before this stage

crates/quarto-core/src/stage/stages/user_filters.rs

@@ -470,15 +470,25 @@ fn shortcode_to_lua_args(
         })
         .collect();
 
-    // Extract top-level metadata as string key-value pairs for Lua
+    // Extract top-level metadata as string key-value pairs for Lua.
+    //
+    // Forward scalars of every stringifiable kind, not just strings: a handler
+    // that gates on a boolean/numeric flag (e.g. the `video` shortcode reading
+    // `auto-stretch: false` to decide reveal stretching — bd-5b21rbaq) needs to
+    // see it. Booleans/ints are stringified ("false", "16"); string and
+    // PandocInlines scalars come through `as_plain_text()`. Map/array values
+    // remain dropped (no flat string form).
     let meta_entries: Vec<(String, String)> = if let Some(entries) = metadata.as_map_entries() {
         entries
             .iter()
             .filter_map(|entry| {
-                entry
-                    .value
-                    .as_str()
-                    .map(|v| (entry.key.clone(), v.to_string()))
+                let v = &entry.value;
+                let s = v
+                    .as_bool()
+                    .map(|b| b.to_string())
+                    .or_else(|| v.as_int().map(|n| n.to_string()))
+                    .or_else(|| v.as_plain_text());
+                s.map(|s| (entry.key.clone(), s))
             })
             .collect()
     } else {

crates/quarto-core/src/transforms/shortcode_resolve.rs

@@ -40,6 +40,7 @@ pub mod replay_engine;
 pub mod revealjs_features;
 pub mod revealjs_format;
 pub mod sidebar_pipeline;
+pub mod video_shortcode_preview;
 pub mod website_post_render;
 
 fn main() {}

crates/quarto-core/tests/integration/main.rs

@@ -444,3 +444,57 @@ fn fragment_data_index_passes_through() {
         &html[..html.len().min(1500)]
     );
 }
+
+// ── video shortcode auto-stretch on reveal (bd-5b21rbaq Phase 5) ──────────
+//
+// A `{{< video >}}` on a slide emits a bare `<iframe>` with no size, which
+// reveal renders at the browser default (~300×150). reveal sizes
+// `section > iframe.r-stretch` to fill the slide, so the video shortcode adds
+// `r-stretch` to the reveal iframe — but only when auto-stretch is on AND the
+// author gave no explicit width/height (so explicit sizing always wins).
+
+fn video_iframe_line(html: &str) -> String {
+    html.lines()
+        .find(|l| l.contains("<iframe") && l.contains("youtube.com/embed"))
+        .unwrap_or_else(|| panic!("no youtube iframe in reveal output:\n{html}"))
+        .to_string()
+}
+
+#[test]
+fn video_reveal_default_iframe_gets_r_stretch() {
+    let html = render_revealjs(
+        "---\nformat: revealjs\n---\n\n## Video\n\n{{< video https://youtu.be/sAWFsP0Bbbk >}}\n",
+    );
+    let iframe = video_iframe_line(&html);
+    assert!(
+        iframe.contains("r-stretch"),
+        "default reveal video iframe must carry `r-stretch` so reveal sizes it; iframe:\n{iframe}"
+    );
+}
+
+#[test]
+fn video_reveal_explicit_width_no_r_stretch() {
+    // Explicit author sizing must win — no auto-stretch.
+    let html = render_revealjs(
+        "---\nformat: revealjs\n---\n\n## Video\n\n{{< video https://youtu.be/sAWFsP0Bbbk width=\"640\" >}}\n",
+    );
+    let iframe = video_iframe_line(&html);
+    assert!(
+        !iframe.contains("r-stretch"),
+        "reveal video with explicit width must NOT be stretched; iframe:\n{iframe}"
+    );
+}
+
+#[test]
+fn video_reveal_auto_stretch_false_no_r_stretch() {
+    // The `auto-stretch: false` global opt-out must suppress the class —
+    // requires the Lua meta bridge to forward the boolean to the handler.
+    let html = render_revealjs(
+        "---\nformat: revealjs\nauto-stretch: false\n---\n\n## Video\n\n{{< video https://youtu.be/sAWFsP0Bbbk >}}\n",
+    );
+    let iframe = video_iframe_line(&html);
+    assert!(
+        !iframe.contains("r-stretch"),
+        "reveal video must NOT be stretched when `auto-stretch: false`; iframe:\n{iframe}"
+    );
+}