diff --git a/ROADMAP.md b/ROADMAP.md index 8ea976e6..727e3d2a 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -131,6 +131,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | What landed | |---|---| +| 2026-10-15 | Review › Compare: Reject and Reject All drop revisions without applying them (`review.reject`); Accept All skips what was rejected | | 2026-10-15 | PDF export and rendering: `anchorCtr` centres the drawn text block — right-aligned text no longer ran past the box edge (#736) | | 2026-10-14 | connected MCP rejects unrelated, malformed, incomplete and oversized replies without replay; buffered unsolicited bytes force a fresh connection before sending (#718) | | 2026-10-14 | Empty bulleted and numbered paragraphs show gray markers while editing, including on first click and after Enter. Markers use the list style and stay out of slide shows and exports. | @@ -226,6 +227,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | Change | Summary | |---|---|---| +| 2026-10-15 | trivial | Progress log: Compare Reject and Reject All | | 2026-10-14 | trivial | remote MCP checks reply IDs, envelopes and bounded newline framing (#718) | | 2026-10-14 | minor | Gray list markers on empty paragraphs while editing; caret keeps the list indent before typing | | 2026-10-14 | minor | Community PR wave folded: headline breadth 84% over 191 features (P0 92%, P1 82%, P2 55%) and the feature-area breadths from `cargo xtask parity`; one row per area; Upcoming item 2 merged; per-PR count notes dropped from the progress log. Ready stays ~55%, stage alpha | diff --git a/crates/engine/src/cmd/review.rs b/crates/engine/src/cmd/review.rs index 65933bbd..4d45f658 100644 --- a/crates/engine/src/cmd/review.rs +++ b/crates/engine/src/cmd/review.rs @@ -59,6 +59,15 @@ pub fn specs() -> Vec { cmd!(noundo "review.compare", "Compare", ["Review", "Compare"], None, "{path} | {name, data: base64} — compare with a revised copy → {name, changes}", has_doc, compare), cmd!(query "review.revisions", "Revisions", [], None, "{} → {name, changes: [{kind: inserted|deleted|changed, slide, revised, details}]}", has_doc, revisions), cmd!("review.accept", "Accept Revision", ["Review", "Compare"], None, "{index} | {all: true}", comparing, accept), + cmd!( + noundo "review.reject", + "Reject Revision", + ["Review", "Compare"], + None, + "{index} | {all: true} → {rejected} (drop revisions without applying them: the open deck stays as it is; not an undo step)", + comparing, + reject + ), cmd!(noundo "review.endCompare", "End Review", ["Review", "Compare"], None, "{}", comparing, end_compare), cmd!(noundo "show.fromStart", "Play from Start", ["Slide Show", "Start Slide Show"], Some("Cmd+Shift+Enter"), "{}", has_slide, |s, _| start(s, 0)), cmd!(noundo "show.fromCurrent", "Play from Current Slide", ["Slide Show", "Start Slide Show"], Some("Cmd+Enter"), "{}", has_slide, |s, _| { @@ -648,7 +657,7 @@ fn revised_bytes(p: &Value) -> Result<(String, Vec)> { fn revisions_json(st: &crate::DocState) -> Result { let c = st.compare.as_ref().ok_or_else(|| bad("review.revisions", "not comparing"))?; let name = std::path::Path::new(&c.name).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| c.name.clone()); - Ok(json!({"name": name, "changes": crate::compare::diff(&st.doc, &c.revised, c.by_position), "byPosition": c.by_position})) + Ok(json!({"name": name, "changes": crate::compare::pending(&st.doc, c), "byPosition": c.by_position})) } fn compare(s: &mut Session, p: &Value) -> Result { @@ -656,7 +665,7 @@ fn compare(s: &mut Session, p: &Value) -> Result { let revised = super::file::open_presentation(&name, &bytes)?; let st = s.doc_mut()?; let by_position = crate::compare::unrelated(&st.doc, &revised); - st.compare = Some(crate::compare::Compare { name, revised: Arc::new(revised), by_position }); + st.compare = Some(crate::compare::Compare { name, revised: Arc::new(revised), by_position, rejected: Default::default() }); s.ui_requests.push(UiRequest::Pane { id: "revisions".into() }); revisions_json(s.doc()?) } @@ -666,7 +675,8 @@ fn revisions(s: &mut Session, _: &Value) -> Result { } fn accept(s: &mut Session, p: &Value) -> Result { - let (rev, by_pos) = s.doc()?.compare.as_ref().map(|c| (c.revised.clone(), c.by_position)).ok_or_else(|| bad("review.accept", "not comparing"))?; + let cmp = s.doc()?.compare.clone().ok_or_else(|| bad("review.accept", "not comparing"))?; + let rev = cmp.revised.clone(); let all = bool_param(p, "all").unwrap_or(false); let index = usize_param(p, "index"); if !all && index.is_none() { @@ -677,7 +687,7 @@ fn accept(s: &mut Session, p: &Value) -> Result { // Each accept shifts indices, so the list is recomputed; bounded by the slide counts. let limit = doc.slides.len().saturating_add(rev.slides.len()).saturating_add(1); for _ in 0..limit { - let changes = crate::compare::diff(doc, &rev, by_pos); + let changes = crate::compare::pending(doc, &cmp); let c = if all { changes.into_iter().find(|c| !(c.kind == crate::compare::ChangeKind::Deleted && doc.slides.len() <= 1)) } else { @@ -700,6 +710,24 @@ fn accept(s: &mut Session, p: &Value) -> Result { }) } +fn reject(s: &mut Session, p: &Value) -> Result { + let all = bool_param(p, "all").unwrap_or(false); + let index = usize_param(p, "index"); + if !all && index.is_none() { + return Err(bad("review.reject", "give `index` or `all`")); + } + let st = s.doc_mut()?; + let doc = st.doc.clone(); + let c = st.compare.as_mut().ok_or_else(|| bad("review.reject", "not comparing"))?; + let pending = crate::compare::pending(&doc, c); + let picked: Vec<_> = if all { pending } else { pending.into_iter().nth(index.unwrap_or(0)).into_iter().collect() }; + if picked.is_empty() { + return Err(bad("review.reject", "no such revision")); + } + c.rejected.extend(picked.iter().map(|ch| crate::compare::ChangeKey::of(&doc, ch))); + Ok(json!({"rejected": picked.len()})) +} + fn end_compare(s: &mut Session, _: &Value) -> Result { s.doc_mut()?.compare = None; ok() diff --git a/crates/engine/src/compare.rs b/crates/engine/src/compare.rs index 93469f06..b24d9649 100644 --- a/crates/engine/src/compare.rs +++ b/crates/engine/src/compare.rs @@ -1,5 +1,5 @@ //! Review › Compare: the differences between the open presentation and a revised copy of it, slide -//! by slide, and accepting a revised slide into the open one. +//! by slide, and accepting a revised slide into the open one or rejecting it. //! //! Slides match by id (a revised copy of the same deck keeps them); when the two decks share no //! slide id at all, by position. Shapes match by id, then by name. @@ -18,6 +18,34 @@ pub struct Compare { /// Slides match by position (the decks shared no slide id when the compare started); fixed /// for the session so accepting slides doesn't switch the matching half way. pub by_position: bool, + /// Revisions rejected: no longer listed and never accepted, the open deck left as it is. + pub rejected: HashSet, +} + +/// What a revision is about, stable while slides move: the open slide (for changed and deleted) +/// and the revised slide (for changed and inserted). +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub struct ChangeKey { + kind: ChangeKind, + open: Option, + revised: Option, +} + +impl ChangeKey { + pub fn of(doc: &Presentation, change: &Change) -> ChangeKey { + let open = match change.kind { + ChangeKind::Inserted => None, + ChangeKind::Changed | ChangeKind::Deleted => doc.slides.get(change.slide).map(|s| s.id), + }; + ChangeKey { kind: change.kind, open, revised: change.revised } + } +} + +/// The revisions still to review: [`diff`] without the rejected ones. +pub fn pending(doc: &Presentation, c: &Compare) -> Vec { + let mut out = diff(doc, &c.revised, c.by_position); + out.retain(|ch| !c.rejected.contains(&ChangeKey::of(doc, ch))); + out } /// Whether `cur` and `rev` share no slide id (then slides match by position). @@ -26,7 +54,7 @@ pub fn unrelated(cur: &Presentation, rev: &Presentation) -> bool { !cur.slides.iter().any(|s| ids.contains(&s.id)) } -#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize)] +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize)] #[serde(rename_all = "camelCase")] pub enum ChangeKind { /// Only in the revised deck. @@ -434,6 +462,50 @@ mod tests { assert!(s.execute("review.accept", &json!({"all": true})).is_err()); } + #[test] + fn reject_keeps_the_open_deck_and_accept_all_skips_rejected_revisions() { + let (mut s, rev, n) = decks(); + s.execute("review.compare", &json!({"name": "rev.deckcraft", "data": rev})).unwrap(); + let before = s.execute("review.revisions", &json!({})).unwrap()["changes"].as_array().unwrap().clone(); + let ins = before.iter().position(|c| c["kind"] == "inserted").unwrap(); + let doc = s.doc().unwrap().doc.clone(); + let undo = s.doc().unwrap().history.undo.len(); + assert_eq!(s.execute("review.reject", &json!({"index": ins})).unwrap()["rejected"], 1); + assert!(std::sync::Arc::ptr_eq(&doc, &s.doc().unwrap().doc), "rejecting leaves the open deck as it is"); + let left = s.execute("review.revisions", &json!({})).unwrap(); + assert_eq!(kinds(&left).len(), before.len() - 1); + assert!(!kinds(&left).contains(&"inserted".into()), "{left}"); + // Accept All takes the rest, never the rejected slide. + s.execute("review.accept", &json!({"all": true})).unwrap(); + let st = s.doc().unwrap(); + assert_eq!(st.doc.slides.len(), n - 1, "the deleted slide goes, the rejected insertion never comes"); + assert!(!st.doc.slides.iter().any(|x| super::slide_title(x) == "Brand new")); + assert!(s.execute("review.revisions", &json!({})).unwrap()["changes"].as_array().unwrap().is_empty()); + s.execute("edit.undo", &json!({})).unwrap(); + assert!(std::sync::Arc::ptr_eq(&doc, &s.doc().unwrap().doc)); + assert_eq!(s.doc().unwrap().history.undo.len(), undo, "rejecting is not an undo step"); + // Reject All: nothing left to review, the deck as it was; Accept All then has nothing to take. + assert_eq!(s.execute("review.reject", &json!({"all": true})).unwrap()["rejected"], before.len() - 1); + assert!(s.execute("review.revisions", &json!({})).unwrap()["changes"].as_array().unwrap().is_empty()); + assert!(s.execute("review.accept", &json!({"all": true})).is_err()); + assert!(std::sync::Arc::ptr_eq(&doc, &s.doc().unwrap().doc)); + assert!(s.execute("review.reject", &json!({"all": true})).is_err(), "nothing left to reject"); + assert!(s.execute("review.reject", &json!({})).is_err()); + s.execute("review.endCompare", &json!({})).unwrap(); + assert!(s.execute("review.reject", &json!({"index": 0})).is_err()); + } + + #[test] + fn rejecting_one_revision_keeps_the_others_in_order() { + let (mut s, rev, _) = decks(); + s.execute("review.compare", &json!({"name": "rev.deckcraft", "data": rev})).unwrap(); + let before = s.execute("review.revisions", &json!({})).unwrap()["changes"].as_array().unwrap().clone(); + assert!(s.execute("review.reject", &json!({"index": before.len()})).is_err()); + s.execute("review.reject", &json!({"index": 0})).unwrap(); + let left = s.execute("review.revisions", &json!({})).unwrap()["changes"].as_array().unwrap().clone(); + assert_eq!(left.as_slice(), &before[1..], "the rejected one goes, the others stay in order"); + } + #[test] fn unrelated_decks_match_by_position_and_bad_input_fails() { let mut s = Session::with_new(); diff --git a/crates/ui-egui/src/compare_tests.rs b/crates/ui-egui/src/compare_tests.rs index 9108b4de..01a40aca 100644 --- a/crates/ui-egui/src/compare_tests.rs +++ b/crates/ui-egui/src/compare_tests.rs @@ -50,6 +50,18 @@ fn pane_opens_lists_changes_and_accept_all_applies_them() { assert!(h.state().session.active().unwrap().compare.is_none()); } +#[test] +fn reject_all_empties_the_pane_and_keeps_the_deck() { + let (mut h, n) = harness(); + let doc = h.state().session.active().unwrap().doc.clone(); + h.get_by_label_contains("Reject All").click(); + h.run_steps(3); + let st = h.state().session.active().unwrap(); + assert!(std::sync::Arc::ptr_eq(&doc, &st.doc), "nothing applied"); + assert_eq!(st.doc.slides.len(), n); + assert!(h.query_by_label("No differences left.").is_some()); +} + #[test] #[ignore = "writes a screenshot for review"] fn screenshot() { diff --git a/crates/ui-egui/src/panes.rs b/crates/ui-egui/src/panes.rs index 39bab078..2ab3120d 100644 --- a/crates/ui-egui/src/panes.rs +++ b/crates/ui-egui/src/panes.rs @@ -1164,7 +1164,8 @@ fn animation(app: &mut SlideApp, ui: &mut Ui) { let _ = paint_anim_tile; } -/// Review › Compare: the revised copy's slide-level changes; click to go there, Accept to take it. +/// Review › Compare: the revised copy's slide-level changes; click to go there, Accept to take it, +/// Reject to drop it. fn revisions(app: &mut SlideApp, ui: &mut Ui) { let t = Tokens::get(ui.ctx()); let Ok(v) = app.session.execute("review.revisions", &json!({})) else { @@ -1185,9 +1186,14 @@ fn revisions(app: &mut SlideApp, ui: &mut Ui) { } return; } - if ui.button(format!("Accept All ({})", changes.len())).clicked() { - run(app, "review.accept", json!({"all": true})); - } + ui.horizontal(|ui| { + if ui.button(format!("Accept All ({})", changes.len())).clicked() { + run(app, "review.accept", json!({"all": true})); + } + if ui.button(format!("Reject All ({})", changes.len())).clicked() { + run(app, "review.reject", json!({"all": true})); + } + }); ui.separator(); for (i, c) in changes.iter().enumerate() { let slide = c["slide"].as_u64().unwrap_or(0); @@ -1206,6 +1212,9 @@ fn revisions(app: &mut SlideApp, ui: &mut Ui) { run(app, "slide.go", json!({"index": (slide as usize).min(max)})); } ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + if ui.small_button("Reject").clicked() { + run(app, "review.reject", json!({"index": i})); + } if ui.small_button("Accept").clicked() { run(app, "review.accept", json!({"index": i})); } diff --git a/docs/gaps.md b/docs/gaps.md index 4e564951..6ee678da 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -273,7 +273,7 @@ Zoom: live thumbnails, the zoom-in animation and the native `p166:sldZm` element artistic effects (3–6 h for the other 15 of 22; 7 approximated and round-tripped), remove background (6–10 h; needs a model or a classical matte), insert screenshot (2–3 h), record audio/screen and record slide show (8–12 h), compare presentations beyond slide-level accept -(reject, per-object accept, markers on the slide: 3–5 h), password protection (4–6 h, ECMA-376 +and reject (per-object accept and reject, markers on the slide: 2–4 h), password protection (4–6 h, ECMA-376 agile encryption), ink to text and math beyond Ink to Shape (6–10 h), 3-D models and stock content (6–10 h; Insert › Icons places a Lucide stroke set). @@ -355,6 +355,7 @@ Detail in [localization-parity.md](localization-parity.md). | Date | Change | Summary | |---|---|---| +| 2026-10-15 | trivial | Compare: slide-level Reject and Reject All landed; the entry keeps per-object accept and reject and the slide markers | | 2026-10-14 | trivial | Fixed #718: remote MCP no longer accepts unrelated or buffered stale success; loopback regressions cover invalid envelopes, incomplete and oversized frames | | 2026-10-14 | minor | Community PR wave folded: one entry each for the soak tests (engine and UI), the PPTX corpus (seven owned corpus files), edit points / text warp / 3-D, proofing (Hunspell spelling, thesaurus, AutoCorrect), the smaller missing rows, dialogs (41 vs ~60; Print and Insert Object left) and pen/trackpad input; ranked rows 3, 4 and 10 de-duplicated (comment threads #70 and the pane timeline are done); the fixed command-palette entry (#79) removed | | 2026-10-13 | minor | EMF/WMF rectangle clip regions (`INTERSECTCLIPRECT`, `EXCLUDECLIPRECT`, `EXTSELECTCLIPRGN`) limit later drawing. EMF+ records remain |