Skip to content

Save: File > Save no longer writes over the file the document was read from through a file written beside the path - #7698

Merged
bflatastic merged 1 commit into
storytold:mainfrom
dotCooCoo:fix/save-checks-files-beside-read-file
Nov 15, 2026
Merged

bflatastic merged 1 commit into
storytold:mainfrom
dotCooCoo:fix/save-checks-files-beside-read-file

Conversation

@dotCooCoo

Copy link
Copy Markdown
Contributor

What and why

Follow-up to #6751.

File > Save to the document's own file planned the save with the loss already acknowledged, so that only its own path was asked about, and then wrote that same plan. Because the plan carried the acknowledgment, the files written beside the path were not checked against the file the document was read from. An EPS saved with Use Artboards writes one file per artboard beside the path. When the document had been opened from one of those artboard files and reading it left things out, File > Save wrote over that file without asking.

Save As and Save a Copy already plan the save again without the acknowledgment before writing (crates/ui-egui/src/io/save.rs, save_suggesting). File > Save now does the same, so a file written beside the path that is the file the document was read from is refused, and nothing in the set is written. The question about the document's own path is unchanged.

How it was tested

New test, shown failing without the change (the artboard file was rewritten): tests::save::file_save_does_not_replace_the_file_read_from_through_a_file_written_beside_the_path.

Run on Linux on 196231d233, based on main at 9ddbf25bef:

  • cargo fmt --all -- --check
  • cargo clippy -p vectorcraft-ui-egui --all-targets -- -D warnings
  • cargo test -p vectorcraft-ui-egui --lib -- tests::save: 75 passed
  • cargo check --workspace --tests

The full cargo xtask ci runs in this pull request's CI.

Checklist

  • cargo xtask ci passes (fmt, clippy, tests, assets, brands, layers, wasm).
  • No panics in shipped code: no unwrap(), expect(), panic!, unreachable!, todo! or
    unimplemented! outside tests, and no unchecked indexing on data from files, parameters or
    messages. Errors are returned as Result and shown to the user
    (why and how).
  • Clean-room: nothing copied from Adobe products or GPL/AGPL code (AGENTS.md).
  • Every new asset has a row in ASSETS.md.

…e file read from

File > Save to the document's own file planned the save with acknowledgeLoss
set, so that only its own path was asked about, and then wrote that plan. With
the acknowledgment in the plan, the files written beside the path (an EPS
saved with Use Artboards writes one per artboard) were not checked against the
file the document was read from. A document opened from one of those artboard
files, with things left out when it was read, was written over by File > Save
without the question. Save As and Save a Copy already plan again without the
acknowledgment before writing; File > Save now does the same.

Follow-up to storytold#6751.
@coderabbitai

coderabbitai Bot commented Nov 14, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 39 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 8f8956f and 196231d.

Files selected for processing: 2

  • crates/ui-egui/src/io/save.rs
  • crates/ui-egui/src/tests/save.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@bflatastic
bflatastic merged commit c18a83f into storytold:main Nov 15, 2026
7 checks passed
@bflatastic

Copy link
Copy Markdown
Member

Merged, thanks @dotCooCoo! It passed its CI checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants