Merge PR #850: Review: reject revisions after Compare, one or all

ROADMAP.md, docs/gaps.md: kept main's lines and the PR's single added line in each table.
This commit is contained in:
aiongg
2026-10-24 06:16:50 +00:00
6 changed files with 135 additions and 11 deletions
+2
View File
@@ -170,6 +170,7 @@ app's JSON control channel (`deckcraft --control PORT`).
| 2026-10-16 | AutoFormat as you type: ordinals (1st, 22nd, 103rd) get superscript suffixes after a space or punctuation; Tools ▸ AutoCorrect and `text.autocorrect {ordinals}` turn it off |
| 2026-10-16 | A PowerPoint Show (`.ppsx`) opened from the command line or the file association starts its slide show, as in PowerPoint; File › Open still opens it for editing (#862) |
| 2026-10-15 | Clicking the frame of a text box or placeholder selects the box whole, so it can be moved, duplicated or deleted; only a click inside puts the caret in the text (#787) |
| 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. |
@@ -280,6 +281,7 @@ app's JSON control channel (`deckcraft --control PORT`).
| 2026-10-16 | trivial | Progress log: responsive mouse-wheel navigation on the fitted canvas |
| 2026-10-21 | trivial | Progress log: theme `spDef` anchor no longer applied to a file's own shapes |
| 2026-10-21 | trivial | Progress log: 3-D sizes in scaled groups stay in points |
| 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 |
+32 -4
View File
@@ -59,6 +59,15 @@ pub fn specs() -> Vec<CommandSpec> {
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, _| {
@@ -655,7 +664,7 @@ fn revised_bytes(p: &Value) -> Result<(String, Vec<u8>)> {
fn revisions_json(st: &crate::DocState) -> Result<Value> {
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<Value> {
@@ -663,7 +672,7 @@ fn compare(s: &mut Session, p: &Value) -> Result<Value> {
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()?)
}
@@ -673,7 +682,8 @@ fn revisions(s: &mut Session, _: &Value) -> Result<Value> {
}
fn accept(s: &mut Session, p: &Value) -> Result<Value> {
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() {
@@ -684,7 +694,7 @@ fn accept(s: &mut Session, p: &Value) -> Result<Value> {
// 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 {
@@ -707,6 +717,24 @@ fn accept(s: &mut Session, p: &Value) -> Result<Value> {
})
}
fn reject(s: &mut Session, p: &Value) -> Result<Value> {
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<Value> {
s.doc_mut()?.compare = None;
ok()
+74 -2
View File
@@ -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<ChangeKey>,
}
/// 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<SlideId>,
revised: Option<usize>,
}
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<Change> {
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();
+12
View File
@@ -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() {
+13 -4
View File
@@ -1189,7 +1189,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 {
@@ -1210,9 +1211,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);
@@ -1231,6 +1237,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}));
}
+2 -1
View File
@@ -276,7 +276,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).
@@ -364,6 +364,7 @@ Detail in [localization-parity.md](localization-parity.md).
| 2026-10-15 | trivial | Native saves retain chart package parts and relationships, including charts in groups; edited chart data still does not export a stale source workbook. Chart editing gaps and scores remain unchanged |
| 2026-10-15 | trivial | PPTX trigger sequences keep later On Click steps instead of converting them to After Previous; mixed starts, independent buttons and paragraph builds have synthetic round-trip regressions |
| 2026-10-16 | trivial | AutoFormat superscripts ordinals (1st, 22nd) as you type, with an AutoCorrect option to turn it off; automatic bulleted lists remain |
| 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 |