Skip to content

feat(home): undo a conversion, its results go to the Recycle Bin / Trash (#553) - #564

Open
cre8ive-marketing wants to merge 4 commits into
KnifeLemon:mainfrom
cre8ive-marketing:feat/553-undo
Open

cre8ive-marketing wants to merge 4 commits into
KnifeLemon:mainfrom
cre8ive-marketing:feat/553-undo

Conversation

@cre8ive-marketing

Copy link
Copy Markdown
Collaborator

Part 2 of #553 (option a, as approved).

What it does

A recent conversion on Home gets an Undo button. Undo moves that job's results to the Recycle Bin / Trash, and the card then says "Undone: the results are in the Recycle Bin / Trash".

Undo on a recent conversion

  • Only while the results are as the job left it. HistoryEntry.Undoable keeps each result's path, size and last write time. The button shows only while every one still matches, so a file edited since, or overwritten by another conversion, is never moved.
  • "Replace the original" (option a): the originals are already in the Recycle Bin / Trash and stay there; the card says "…, and so are the originals they replaced". Feature request: trim transparent borders of PNG images (single & batch, in-place) #332 is unchanged.
  • Never the user's own file: with "Replace the original" + "Only if smaller", the result is the untouched source. FileResult.OriginalInTrash records whether the original really went to the Trash, and a source that didn't is never listed as a result.
  • When the Trash refuses (a file open elsewhere …), the card says which file and why, the entry isn't marked undone, and Undo can be tried again. Results moved by the first try are skipped.
  • An output folder ("Extract") has no Undo: Undo only moves files.
  • Entries from before this have nothing to undo. One field in history.json, no settings change.

New strings (home.undo, home.undo_hint, home.undone, home.undone_original, home.undo_failed, trash.changed) are in all 16 languages. One new icon, Icon.Undo (Material Design "undo").

Tests

  • HistoryEntryTests: a finished job remembers its results for Undo and they survive a reload; a changed or deleted result no longer matches; an old entry has nothing to undo.
  • ReplaceOriginalTests: a result in the original's place is undoable and says the original is in the Trash; when the Trash refuses, only the result beside it is undoable; "Only if smaller" leaves an unchanged original that Undo must never touch.
  • RecentConversionTests: Undo moves the results, saves the entry as undone, shows the refusal reason, and offers nothing for a result changed since; the Home screenshots now show the button.
  • dotnet format --verify-no-changes is clean.
  • Full dotnet test: Platform 81, Core 606, CLI 27, Setup 176, Engines 1821 and App 1345 pass, none fail.

Closes part of #553 (Fit to a place comes in its own PR).

…nifeLemon#553)

A recent conversion gets an Undo button while every result is still as the
job left it (same size and write time, saved in history.json). Undo moves the
results to the Recycle Bin / Trash and the card says so. With "Replace the
original" it also says the originals are there; they stay (KnifeLemon#332 unchanged).
A source left as it was ("Only if smaller") is never treated as a result.
@cre8ive-marketing

Copy link
Copy Markdown
Collaborator Author

@KnifeLemon, CI is green on every check here. When you have reviewed it, could you merge it into main?

@KnifeLemon KnifeLemon left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @cre8ive-marketing, this is option a as agreed: Undo only goes through the existing MoveToTrash, which never deletes for good. The size-and-time stamp keeps it away from files that changed. The "Only if smaller" + "Replace the original" case, where the result is the user's own file, is handled and tested. Old history.json entries load and simply have nothing to undo.

  1. A partly done Undo gets stuck. HomePageViewModel.cs:138 checks Unchanged() and moves file by file. If the second result changed, or the Trash refuses it ("in use"), the first is already in the Trash and the entry isn't marked. After the next HistoryChanged the card is rebuilt, and line 61 requires every result to still exist, so CanUndo is false: half the results are in the Trash, with no Undo button and no "Undone" text. Please check every result before moving any, record which ones were moved (e.g. a Trashed flag on HistoryOutput, saved even when a later one is refused), and base CanUndo on the results not yet moved.
  2. Undo runs on the UI thread. On Windows, RecycleBin.Move starts and joins an STA thread for each file, so a job with hundreds of results ("Save every frame", PDF pages) freezes the window until it's done. Please make it an async command that moves the files off the UI thread, with the button disabled meanwhile.
  3. Pandoc's picture folder is left behind. Pandoc writes a document's pictures into <name>_files next to it (JobQueue.cs:974-975), and that folder isn't in Outputs, so Undo moves the .md/.html to the Trash and leaves its pictures. Either move that folder too, if it's still there, or give such jobs no Undo, as you did for Extract.
  4. Optional: Undoable repeats every path Outputs already holds. For a frame-sequence job that's thousands of extra lines in history.json per entry; no Undo above a few hundred results would keep it small.

…UI thread

From the review of KnifeLemon#564:
- Every result is checked before any moves; a changed one stops Undo with nothing moved.
- Each result moved is saved as Trashed, also when the Trash then refuses a later one, so the card keeps offering Undo for the rest instead of getting stuck.
- The files move on a background thread; the button is disabled meanwhile.
- A document whose pictures Pandoc wrote into "<name>_files" gets no Undo, rather than leaving the pictures behind.
- More than 500 results get no Undo, so history.json stays small.
@cre8ive-marketing

Copy link
Copy Markdown
Collaborator Author

@KnifeLemon, thanks for the review. All four points are in, pushed as 32efd75 and 363daea, with main merged in:

  1. Partly done Undo: every result is checked before any moves, so a changed one stops it with nothing moved. Each result moved is saved as Trashed on its HistoryOutput, also when the Trash then refuses a later one (SaveHistory(notify: false), so the card can say why). CanUndo looks only at the results not moved yet, so the card keeps offering Undo for the rest, also after a reload. The test covers it: the Trash refuses the second file, the first stays moved and saved, and a second Undo moves only the second.
  2. UI thread: UndoAsync moves the files on a background thread. The async command keeps the button disabled while it runs.
  3. Pandoc's _files folder: a job where a result has a <name>_files folder beside it gets no Undo, as Extract doesn't. Tested with the fake engine's picture folder.
  4. Size: more than 500 results (HistoryEntry.MaxUndoable) get no Undo, so a frame-sequence job adds nothing to history.json.

German strings for the new keys are included, since #541 came in on main.

Full dotnet test: Platform 81, Core 608, CLI 30, Setup 181, Engines 1822 and App 1370 pass, none fail.

@cre8ive-marketing

Copy link
Copy Markdown
Collaborator Author

@KnifeLemon, CI is green on every check after the review fixes. Could you take another look and merge it into main if it is good?

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

Labels

feature New features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants