Skip to content

Show how far accept all has got - #874

Merged
SimonCropp merged 2 commits into
mainfrom
accept-all-progress
Sep 22, 2026
Merged

SimonCropp merged 2 commits into
mainfrom
accept-all-progress

Conversation

@SimonCropp

Copy link
Copy Markdown
Member

Accept all takes as long as the queue is long - every snapshot is a read, a patch and a rewrite of its source under a cross process mutex, and a move can be retried for seconds while a diff tool lets go of it - and nothing said so. A viewer that owned the queue ran the whole batch as one transition on its render thread, so the window stopped painting until it was done and then emptied at once. A viewer displaying the tray's queue said "Waiting for the queue owner." over a list that did not move: the tray applied everything before recording anything, and the viewer only listed once its accept-all came back. A batch longer than fifteen seconds then reported the owner as no longer running while it was still accepting.

The viewer now carries out an accept-all an entry at a time. BeginAcceptAll records an AcceptBatch, and AcceptAllRunner claims an entry under the session's lock, applies it outside, and records it under the lock again, so between entries the window draws the queue shrinking and the status line says "Accepting 12 of 40". A batch started in the window runs on a worker, one the tray asked for runs on the listener thread, and only one drives at a time. Snapshots go before files, since whether a delete is held turns on how the snapshots went. ViewerSession.Apply(AcceptAll) is the same steps back to back, which is what the existing tests already drive. While a batch runs, Accept, Discard and Accept all are disabled and their keys and menu items do nothing; scrolling, selecting, folding and copying carry on.

The tray records each snapshot as it lands, with InlineQueue.AcceptInBatch - the batch's rules, one entry at a time, which AcceptAll over a list of outcomes is now made of - and its listings carry how far the batch has got, the tracked files included. The listing gained an optional progress: done|total line, which a reader that predates it skips. OwnerLink lists beside a forwarded command rather than after it, so a window attached to the tray follows the batch whichever surface started it, and a finished command's message still arrives with the listing that shows its effect. Forwarded commands, and the tray's accept-all against a viewer, now wait five minutes rather than fifteen seconds, since the listing is what says whether the owner is still there.

Checked against the real Windows viewer, 40 queued snapshots with one file's patch mutex held for five seconds: the window showed "Accepting 6 of 40" over the 35 still pending, with the acting buttons disabled, and its listing answered throughout; all 40 landed once the mutex was let go. On main the same run showed "Pending (40)" and "lines 1-1 of 1" with five files already written, and the listing did not answer until the mutex was let go.

AcceptAllProgressTests steps a batch and holds one apply part way through, over the session, the runner, the wire and an attached viewer's polling loop; InlineScreenTests.AcceptAllInProgress is the screen part way through; TrayViewerSyncTest.AViewerAttachedToTheTrayFollowsItsAcceptAll is the arrangement the tray sets up at login, end to end over a real socket.

Group accepts ("Accept all in ...") are still one transition. They drop stale entries where accept-all keeps them, so moving them onto the batch is its own change.

Accept all takes as long as the queue is long - every snapshot is a
read, a patch and a rewrite of its source under a cross process mutex,
and a move can be retried for seconds while a diff tool lets go of it -
and nothing said so. A viewer that owned the queue ran the whole batch
as one transition on its render thread, so the window stopped painting
until it was done and then emptied at once. A viewer displaying the
tray's queue said "Waiting for the queue owner." over a list that did
not move: the tray applied everything before recording anything, and
the viewer only listed once its accept-all came back. A batch longer
than fifteen seconds then reported the owner as no longer running while
it was still accepting.

The viewer now carries out an accept-all an entry at a time.
BeginAcceptAll records an AcceptBatch, and AcceptAllRunner claims an
entry under the session's lock, applies it outside, and records it
under the lock again, so between entries the window draws the queue
shrinking and the status line says "Accepting 12 of 40". A batch
started in the window runs on a worker, one the tray asked for runs on
the listener thread, and only one drives at a time. Snapshots go before
files, since whether a delete is held turns on how the snapshots went.
ViewerSession.Apply(AcceptAll) is the same steps back to back, which is
what the existing tests already drive. While a batch runs, Accept,
Discard and Accept all are disabled and their keys and menu items do
nothing; scrolling, selecting, folding and copying carry on.

The tray records each snapshot as it lands, with
InlineQueue.AcceptInBatch - the batch's rules, one entry at a time,
which AcceptAll over a list of outcomes is now made of - and its
listings carry how far the batch has got, the tracked files included.
The listing gained an optional "progress: done|total" line, which a
reader that predates it skips. OwnerLink lists beside a forwarded
command rather than after it, so a window attached to the tray follows
the batch whichever surface started it, and a finished command's
message still arrives with the listing that shows its effect. Forwarded
commands, and the tray's accept-all against a viewer, now wait five
minutes rather than fifteen seconds, since the listing is what says
whether the owner is still there.

Checked against the real Windows viewer, 40 queued snapshots with one
file's patch mutex held for five seconds: the window showed "Accepting
6 of 40" over the 35 still pending, with the acting buttons disabled,
and its listing answered throughout; all 40 landed once the mutex was
let go. On main the same run showed "Pending (40)" and "lines 1-1 of 1"
with five files already written, and the listing did not answer until
the mutex was let go.

Group accepts ("Accept all in ...") are still one transition; they
drop stale entries where accept-all keeps them, so moving them onto the
batch is its own change.
ViewerLaunchTests.AcceptAllOverALongQueue opens the viewer on 503
pending snapshots for a person to press Accept all on: two solutions of
ten classes, twenty five multi line snapshots a class, so every accept
moves the call sites below it, plus three conflicts that keep the
window open afterwards to show what the batch said.

A fast disk accepts all of it in a second or two, too quick to watch,
so the test holds the class halfway down the batch by the mutex
InlineApplier waits on, the way another process writing it would. The
count stops there for six seconds while the window keeps answering,
then carries on. It is taken only once a batch has started.
@SimonCropp SimonCropp added this to the 20.4.0 milestone Sep 22, 2026
@SimonCropp
SimonCropp merged commit edf14c2 into main Sep 22, 2026
8 checks passed
@SimonCropp
SimonCropp deleted the accept-all-progress branch September 22, 2026 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant