Merge pull request #5169 from dotCooCoo/fix/actions-current-set-after-restart

Actions: the current set is kept after a restart
This commit is contained in:
BFlat
2026-11-02 14:48:19 +07:00
committed by GitHub
5 changed files with 271 additions and 4 deletions
+198 -1
View File
@@ -171,6 +171,11 @@ fn merge_changes(base: &serde_json::Value, mine: &serde_json::Value, on_disk: se
let (Some(base), Some(mine), Value::Object(mut out)) = (base.as_object(), mine.as_object(), on_disk) else {
return mine.clone();
};
// The current Actions set (`action_set`, an index) is this window's when this window chose
// another one or the other window did not; otherwise it is the other window's.
let use_mine = chose_another_action_set(base, mine) || !chose_another_action_set(base, &out);
// The list the saved current set was chosen from.
let chosen_from = if use_mine { mine.get("action_sets").cloned() } else { out.get("action_sets").cloned() };
let keys: std::collections::BTreeSet<&String> = base.keys().chain(mine.keys()).collect();
for key in keys {
if PASSING.contains(&key.as_str()) {
@@ -180,7 +185,7 @@ fn merge_changes(base: &serde_json::Value, mine: &serde_json::Value, on_disk: se
continue;
}
let (was, now) = (base.get(key), mine.get(key));
if was == now {
if was == now || key == "action_set" {
continue;
}
if key == "engine_prefs"
@@ -209,9 +214,95 @@ fn merge_changes(base: &serde_json::Value, mine: &serde_json::Value, on_disk: se
}
}
}
if use_mine {
match mine.get("action_set") {
Some(v) => {
out.insert("action_set".to_string(), v.clone());
}
None => {
out.remove("action_set");
}
}
}
keep_current_action_set(&mut out, chosen_from.as_ref());
Value::Object(out)
}
/// Point the saved current Actions set (`action_set`) at the same set in the merged
/// `action_sets` when the merge took the index from one window and the list from another
/// ([`find_action_set`]). With no such set, no set is current.
fn keep_current_action_set(out: &mut serde_json::Map<String, serde_json::Value>, chosen_from: Option<&serde_json::Value>) {
use serde_json::Value;
let Some(i) = out.get("action_set").and_then(Value::as_u64).and_then(|i| usize::try_from(i).ok()) else { return };
let merged = out.get("action_sets");
if merged == chosen_from {
return;
}
match find_action_set(chosen_from, i, merged) {
Some(j) => {
out.insert("action_set".to_string(), Value::from(j));
}
None => {
out.remove("action_set");
}
}
}
/// Was another Actions set made current from `base` to `now`? Recording into the current set,
/// renaming it, deleting it, or deleting a set before it changes the list or the index without
/// choosing another set.
fn chose_another_action_set(base: &serde_json::Map<String, serde_json::Value>, now: &serde_json::Map<String, serde_json::Value>) -> bool {
use serde_json::Value;
let index = |v: &serde_json::Map<String, Value>| v.get("action_set").and_then(Value::as_u64).and_then(|i| usize::try_from(i).ok());
let (was, sets) = (base.get("action_sets"), now.get("action_sets"));
let (Some(i), Some(k)) = (index(base), index(now)) else {
return match (index(base), index(now)) {
(None, None) => false,
// No set is current: another set was chosen only if the current one is still there.
(Some(i), None) => find_action_set(was, i, sets).is_some(),
_ => true,
};
};
if was == sets {
return i != k;
}
if let Some(j) = find_action_set(was, i, sets) {
return j != k;
}
// The set at the same place changed while every other set stayed as it was: it was edited.
let edited_in_place = was
.and_then(Value::as_array)
.zip(sets.and_then(Value::as_array))
.is_some_and(|(was, sets)| was.len() == sets.len() && was.iter().zip(sets).enumerate().all(|(j, (a, b))| j == i || a == b));
!(i == k && edited_in_place)
}
/// Where the Actions set at index `i` of `old` is in `new` (two `action_sets` lists): found by
/// its contents when no other set in either list has the same contents, otherwise by its name when
/// no other set in either list has it.
fn find_action_set(old: Option<&serde_json::Value>, i: usize, new: Option<&serde_json::Value>) -> Option<usize> {
use serde_json::Value;
let (old, new) = (old?.as_array()?, new?.as_array()?);
let set = old.get(i)?;
let only = |list: &[Value], same: &dyn Fn(&Value) -> bool| {
let mut found = list.iter().enumerate().filter(|(_, s)| same(s));
match (found.next(), found.next()) {
(Some((j, _)), None) => Some(j),
_ => None,
}
};
let same_contents = |s: &Value| s == set;
if only(old, &same_contents).is_some()
&& let Some(j) = only(new, &same_contents)
{
return Some(j);
}
let name = set.get("name")?;
let same_name = |s: &Value| s.get("name") == Some(name);
only(old, &same_name)?;
only(new, &same_name)
}
/// The preferences file's contents: the UI state with the engine preferences.
#[cfg(test)]
pub(crate) fn prefs_bytes(app: &VectorcraftApp) -> serde_json::Result<Vec<u8>> {
@@ -274,6 +365,112 @@ mod tests {
assert!(!same_lasting(&base, &mine));
}
/// One window deleted the first Actions set and saved; this one made the second set current.
/// The saved index follows that set to its place in the merged list.
#[test]
fn the_current_action_set_follows_its_set_in_the_merged_list() {
let set = |name: &str| json!({"name": name, "actions": []});
let base = json!({"action_sets": [set("A"), set("B"), set("C")], "action_set": 0});
let on_disk = json!({"action_sets": [set("B"), set("C")]});
let mine = json!({"action_sets": [set("A"), set("B"), set("C")], "action_set": 1});
assert_eq!(merge_changes(&base, &mine, on_disk), json!({"action_sets": [set("B"), set("C")], "action_set": 0}));
// The other window also recorded into B: B is found by its name.
let on_disk = json!({"action_sets": [{"name": "B", "actions": [{"name": "Action 1", "steps": []}]}, set("C")]});
let merged = merge_changes(&base, &mine, on_disk);
assert_eq!(merged["action_set"], json!(0));
// The other window deleted B: no set is current.
let on_disk = json!({"action_sets": [set("A"), set("C")]});
assert_eq!(merge_changes(&base, &mine, on_disk), json!({"action_sets": [set("A"), set("C")]}));
}
/// This window deleted the first set while the index saved by the other window still stands.
#[test]
fn the_other_windows_current_action_set_follows_its_set() {
let set = |name: &str| json!({"name": name, "actions": []});
let base = json!({"action_sets": [set("A"), set("B"), set("C")]});
let on_disk = json!({"action_sets": [set("A"), set("B"), set("C")], "action_set": 2});
let mine = json!({"action_sets": [set("B"), set("C")]});
assert_eq!(merge_changes(&base, &mine, on_disk), json!({"action_sets": [set("B"), set("C")], "action_set": 1}));
}
/// The other window replaced or renamed the set this one made current (the saved sets do not
/// tell the two apart): no set is current.
#[test]
fn a_set_in_the_place_of_the_current_set_is_not_made_current() {
let set = |name: &str| json!({"name": name, "actions": []});
let base = json!({"action_sets": [set("A"), set("B"), set("C")]});
let on_disk = json!({"action_sets": [set("A"), set("D"), set("C")]});
let mine = json!({"action_sets": [set("A"), set("B"), set("C")], "action_set": 1});
assert_eq!(merge_changes(&base, &mine, on_disk).get("action_set"), None);
let on_disk = json!({"action_sets": [set("A"), set("C"), set("D")]});
assert_eq!(merge_changes(&base, &mine, on_disk).get("action_set"), None);
}
/// This window deleted a set and made the set now at the old index current: its choice is
/// saved, though the index is the same number.
#[test]
fn a_new_current_set_at_the_same_index_is_saved() {
let set = |name: &str| json!({"name": name, "actions": []});
let base = json!({"action_sets": [set("A"), set("B"), set("C")], "action_set": 1});
let on_disk = json!({"action_sets": [{"name": "A", "actions": [{"name": "Action 1", "steps": []}]}, set("B"), set("C")], "action_set": 1});
let mine = json!({"action_sets": [set("B"), set("C")], "action_set": 1});
assert_eq!(merge_changes(&base, &mine, on_disk), json!({"action_sets": [set("B"), set("C")], "action_set": 1}));
}
/// Recording into the current set does not choose it again: the other window's choice stays.
#[test]
fn recording_into_the_current_set_keeps_the_other_windows_choice() {
let set = |name: &str| json!({"name": name, "actions": []});
let recorded = json!({"name": "A", "actions": [{"name": "Action 1", "steps": []}]});
let base = json!({"action_sets": [set("A"), set("B"), set("C")], "action_set": 0});
let on_disk = json!({"action_sets": [set("A"), set("B"), set("C")], "action_set": 2});
let mine = json!({"action_sets": [recorded.clone(), set("B"), set("C")], "action_set": 0});
assert_eq!(merge_changes(&base, &mine, on_disk), json!({"action_sets": [recorded, set("B"), set("C")], "action_set": 2}));
}
/// Renaming the current set does not choose it again: the other window's choice stays. With no
/// other choice, the renamed set stays current.
#[test]
fn renaming_the_current_set_keeps_the_other_windows_choice() {
let set = |name: &str| json!({"name": name, "actions": []});
let base = json!({"action_sets": [set("A"), set("B"), set("C")], "action_set": 0});
let on_disk = json!({"action_sets": [set("A"), set("B"), set("C")], "action_set": 1});
let mine = json!({"action_sets": [set("A2"), set("B"), set("C")], "action_set": 0});
assert_eq!(merge_changes(&base, &mine, on_disk)["action_set"], json!(1));
assert_eq!(merge_changes(&base, &mine, base.clone())["action_set"], json!(0));
}
/// Deleting the current set does not override the other window's choice.
#[test]
fn deleting_the_current_set_keeps_the_other_windows_choice() {
let set = |name: &str| json!({"name": name, "actions": []});
let base = json!({"action_sets": [set("A"), set("B")], "action_set": 0});
let on_disk = json!({"action_sets": [set("A"), set("B")], "action_set": 1});
let mine = json!({"action_sets": [set("B")]});
assert_eq!(merge_changes(&base, &mine, on_disk), json!({"action_sets": [set("B")], "action_set": 0}));
assert_eq!(merge_changes(&base, &mine, base.clone()), json!({"action_sets": [set("B")]}));
}
/// Deleting a set before the current one moves its index without choosing it again.
#[test]
fn deleting_a_set_before_the_current_one_keeps_it_current() {
let set = |name: &str| json!({"name": name, "actions": []});
let base = json!({"action_sets": [set("A"), set("B"), set("C")], "action_set": 2});
let mine = json!({"action_sets": [set("B"), set("C")], "action_set": 1});
assert_eq!(merge_changes(&base, &mine, base.clone()), json!({"action_sets": [set("B"), set("C")], "action_set": 1}));
}
/// Two sets with the same contents: when the other window deleted the one this window made
/// current, the one left is not made current in its place.
#[test]
fn a_set_with_the_same_contents_does_not_take_the_place_of_the_current_set() {
let set = |name: &str| json!({"name": name, "actions": []});
let base = json!({"action_sets": [set("A"), set("A"), set("C")]});
let on_disk = json!({"action_sets": [set("A"), set("C")]});
let mine = json!({"action_sets": [set("A"), set("A"), set("C")], "action_set": 1});
assert_eq!(merge_changes(&base, &mine, on_disk), json!({"action_sets": [set("A"), set("C")]}));
}
#[test]
fn a_file_that_is_not_an_object_is_replaced() {
let base = json!({"workspace": "Essentials"});
+67
View File
@@ -39,6 +39,14 @@ fn next_set_id() -> u64 {
NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed)
}
/// Serde reader for the saved current set ([`crate::UiState::action_set`]): a whole number is the
/// set's index, and any other value is no current set. An index past the last set is ignored where
/// the panel reads it.
pub fn deserialize_current_set<'de, D: serde::Deserializer<'de>>(d: D) -> Result<Option<usize>, D::Error> {
let v = <Value as serde::Deserialize>::deserialize(d)?;
Ok(v.as_u64().and_then(|i| usize::try_from(i).ok()))
}
/// Built-in actions (our own, handy defaults).
pub fn default_sets() -> Vec<ActionSet> {
let step = |c: &str, p: Value| (c.to_string(), p);
@@ -374,6 +382,12 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut egui::Ui) {
app.ui.action_sets.push(ActionSet::new("Set 1", vec![]));
}
let set = current.filter(|&s| s < app.ui.action_sets.len()).unwrap_or(0);
// With no set current, the recording goes into the first set, which becomes current so
// the panel shows where the recording goes.
if app.ui.action_set != Some(set) {
app.ui.action_set = Some(set);
app.ui.action_sets_selected = vec![set];
}
let n = app.ui.action_sets.get(set).map_or(0, |s| s.actions.len()) + 1;
let start = app.session.journal.len();
// Keep every step from here on until recording stops, however long it runs.
@@ -733,6 +747,59 @@ mod tests {
assert_eq!(ids.len(), 6, "six sets, six ids");
}
/// The current set is saved with the UI preferences: after a restart a recording goes into the
/// set clicked before it, not into Default Actions.
#[test]
fn a_recording_after_a_restart_goes_into_the_set_current_before_it() {
use super::super::tests_appearance::{click, frame_events};
let mut app = app_with_sets();
let ctx = egui::Context::default();
click_set(&ctx, &mut app, "Other", egui::Modifiers::NONE);
let saved = serde_json::to_value(&app.ui).unwrap();
let mut app = VectorcraftApp::new(Session::new(), Default::default());
app.ui = serde_json::from_value(saved).unwrap();
app.run("file.new", json!({"width": 200, "height": 100})).unwrap();
assert_eq!(app.ui.action_set, Some(2), "Other is current after the restart");
let ctx = egui::Context::default();
frame_events(&ctx, &mut app, vec![], show);
let mut bar = crate::toolbar::tests::widget_rects(&ctx, egui::vec2(24.0, 24.0));
bar.sort_by(|a, b| a.left().total_cmp(&b.left()));
let [stop, record, _, _, _] = bar[..] else { panic!("the bottom bar: {bar:?}") };
click(&ctx, &mut app, record.center(), show);
app.run("shape.rectangle", json!({"x": 10, "y": 10, "width": 20, "height": 20})).unwrap();
click(&ctx, &mut app, stop.center(), show);
let counts = app.ui.action_sets.iter().map(|s| s.actions.len()).collect::<Vec<_>>();
assert_eq!(counts, [6, 2, 1], "recorded into Other");
}
/// A saved current set that is not a set index, or is past the last set, leaves no set current;
/// the other preferences still load, and Begin Recording then records into the first set and
/// makes it current.
#[test]
fn a_saved_current_set_that_is_gone_falls_back_to_the_first_set() {
use super::super::tests_appearance::{click, frame_events};
let app = app_with_sets();
let mut saved = serde_json::to_value(&app.ui).unwrap();
saved["action_set"] = json!("Other");
let restored = serde_json::from_value::<crate::state::UiState>(saved.clone()).unwrap();
assert_eq!((restored.action_set, restored.action_sets.len()), (None, 3), "not an index: no set current, the sets load");
saved["action_set"] = json!(9);
let mut app = VectorcraftApp::new(Session::new(), Default::default());
app.ui = serde_json::from_value(saved).unwrap();
app.run("file.new", json!({"width": 200, "height": 100})).unwrap();
let ctx = egui::Context::default();
frame_events(&ctx, &mut app, vec![], show);
let mut bar = crate::toolbar::tests::widget_rects(&ctx, egui::vec2(24.0, 24.0));
bar.sort_by(|a, b| a.left().total_cmp(&b.left()));
let [stop, record, _, _, _] = bar[..] else { panic!("the bottom bar: {bar:?}") };
click(&ctx, &mut app, record.center(), show);
assert_eq!((app.ui.action_set, app.ui.action_sets_selected.clone()), (Some(0), vec![0]), "the set recorded into is current");
app.run("shape.rectangle", json!({"x": 10, "y": 10, "width": 20, "height": 20})).unwrap();
click(&ctx, &mut app, stop.center(), show);
let counts = app.ui.action_sets.iter().map(|s| s.actions.len()).collect::<Vec<_>>();
assert_eq!(counts, [7, 2, 0], "recorded into Default Actions");
}
/// Shift-click on another set's name while an action is selected selects the sets from the
/// current one to it and clears the action.
#[test]
+4 -2
View File
@@ -448,8 +448,10 @@ pub struct UiState {
#[serde(default = "crate::panels::actions::default_sets")]
pub action_sets: Vec<crate::panels::actions::ActionSet>,
/// The current set of the Actions panel (its name clicked, one of its actions clicked, or made
/// with New Set): a recording goes into it and Set Options renames it. Not saved.
#[serde(skip)]
/// with New Set): a recording goes into it and Set Options renames it. Saved as an index into
/// [`UiState::action_sets`], so the same set is current after a restart
/// ([`crate::panels::actions::deserialize_current_set`]).
#[serde(default, skip_serializing_if = "Option::is_none", deserialize_with = "crate::panels::actions::deserialize_current_set")]
pub action_set: Option<usize>,
/// The sets selected in the Actions panel, in index order: the current set and the sets a
/// Shift-click on a set's name added. Delete Selection deletes them. Not saved.
+1
View File
@@ -550,6 +550,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur
- Variables: Lock Variables (`variable.lock`, in the Variables panel's bottom bar and menu) stops variables from being defined, deleted or renamed until they are unlocked, while objects can still be bound and unbound and data sets still change and apply; the lock is saved with the document.
- Batch…: Override Save and Override Export name each file with the extension of the format the step writes, however the step spells the format (`.svg` and `SVG` give `.svg`, `png8` gives `.png`), in `action.batch`.
- Actions panel (record/playback, persisted), generic parameter dialogs for every "…" command.
- Actions panel: the current set is kept with the UI preferences, so after a restart a recording goes into the set that was current, not into the first set. With no set current, Begin Recording records into the first set and makes it current.
- Actions panel: Delete Selection deletes the selected action set with its actions and keeps the other sets ([Actions panel overview](https://helpx.adobe.com/illustrator/desktop/automate-visualize-data/automate-actions/actions-panel-overview.html): "Removes the selected action, command, or action set from the panel"; `ui.actionSet.delete`). VectorCraft's own rules, which the overview does not cover: deleting a set asks first, Shift-click on a set's name selects a range of sets to delete together, the panel menu's Delete does the same, and Default Actions and the last set can be deleted. Undo does not bring a deleted set back.
- Actions panel: the panel menu's Duplicate copies the selected action, else the current action set with all its actions ([Actions panel overview](https://helpx.adobe.com/illustrator/desktop/automate-visualize-data/automate-actions/actions-panel-overview.html): "Makes a copy of the selected action or set."; `ui.actionSet.duplicate`). VectorCraft's own rules, which the overview does not cover: the copy goes right after the original, is named after it with " copy" added and is selected.
- Batch… over data sets names each file after the document and the data set (`action.batch`). It writes `/`, `\`, `:`, `<`, `>`, `"`, `|`, `?`, `*` and control characters in the data set's name as `-`, and adds `-2`, `-3`… to a name an earlier data set used.
+1 -1
View File
@@ -297,7 +297,7 @@ next unit in the order a right-click on a ruler lists them, from Feet back to Po
| Image Trace panel: the Preview checkbox before Trace chooses whether a setting change traces the selected Image Trace object again at once | done | Per the public documentation, Preview shows the result of the current settings and Trace traces with them (<https://web.archive.org/web/20250831090623/https://helpx.adobe.com/illustrator/using/image-trace.html>). Preview is on by default and kept with the UI preferences; Reset to Default leaves it. Off, changes wait in the panel until Trace applies them as one undo step, or until Preview is turned on again. Changes stay with the object they were made for while another object is selected, and come back when it is selected again. `imageTrace.preview {on?}`, `ui.set {imageTracePreview}`; `panels/image_trace.rs` headless tests (`with_preview_off_a_slider_change_waits_for_trace`, `turning_preview_on_traces_the_changes_that_waited`, `preview_turned_on_with_nothing_selected_traces_when_the_object_is_selected_again`, `settings_that_wait_stay_with_their_object`, `settings_that_wait_come_back_when_their_object_is_selected_again`, `settings_that_wait_are_dropped_when_their_object_changed_since`, `preview_is_kept_with_the_preferences_and_reset_to_default_leaves_it`) |
| Toolbar without group labels by default, one or two columns; All Tools drawer by flyout group with shortcuts, its menu (Reset, Basic, Advanced) and Show section | done | #1751; a preference brings the labels back; `toolbar.show` sets the Fill/Stroke, Coloring, Draw Mode and Screen Mode controls |
| Actions panel: a clicked action stays selected for Play Current Selection and Delete Selection | partial | #1853 (part of #1629); editing actions and action shortcuts remain |
| Actions panel: Create New Set (bottom bar, ≡ › New Set…) names a new set and Set Options… renames the current one; a click on a set's name makes it current and a recording goes into the current set ([helpx: Automation with actions](https://helpx.adobe.com/illustrator/using/automation-actions.html)) | partial | #3712; the New Action dialog (name, set menu, shortcut, colour) and dragging actions between sets remain |
| Actions panel: Create New Set (bottom bar, ≡ › New Set…) names a new set and Set Options… renames the current one; a click on a set's name makes it current and a recording goes into the current set ([helpx: Automation with actions](https://helpx.adobe.com/illustrator/using/automation-actions.html)); the current set is kept with the UI preferences, and with no set current a recording goes into the first set, which becomes current | partial | #3712; the New Action dialog (name, set menu, shortcut, colour) and dragging actions between sets remain |
| Actions panel: Delete Selection (bottom bar) deletes the selected set with its actions and keeps the other sets ([Actions panel overview](https://helpx.adobe.com/illustrator/desktop/automate-visualize-data/automate-actions/actions-panel-overview.html): "Removes the selected action, command, or action set from the panel") | partial | `ui.actionSet.delete`; `panels/actions.rs` tests. VectorCraft's own rules, which the overview does not cover: deleting a set asks first (a selected action is still deleted at once), Shift-click on a set's name selects a range of sets, ≡ › Delete does the same as the button, and Default Actions and the last set can be deleted. Undo does not restore a deleted set, and recorded commands cannot be selected or deleted |
| Actions panel: ≡ › Duplicate copies the selected action or set ([Actions panel overview](https://helpx.adobe.com/illustrator/desktop/automate-visualize-data/automate-actions/actions-panel-overview.html): "Makes a copy of the selected action or set.") | partial | `ui.actionSet.duplicate`; `panels/actions.rs` tests. VectorCraft's own rules, which the overview does not cover: the copy goes right after the original, is named after it with " copy" added and is selected, and a copied set holds copies of all its actions. Undo does not remove the copy, and recorded commands cannot be duplicated |
| Properties panel per context (every selection type's Quick Actions) | partial | one clip group: Edit Clipping Path and Edit Contents, at the top of the panel rather than in Quick Actions (G1); the overview's additional controls ([Properties panel overview](https://helpx.adobe.com/illustrator/desktop/get-started/learn-the-basics/properties-panel-overview.html)): Blend Options (one blend selected, with any tool, #4817) and Merge Live Paint (a Live Paint group with more) in Quick Actions, the Width Profile list (Width tool or a variable-width stroke) and an applied effect's Delete icon in Appearance, Corner and Corner Radius for corners selected with Direct Selection; Variable Font was already there |