diff --git a/src/Stampeded/Documents/StartDocumentViewModel.cs b/src/Stampeded/Documents/StartDocumentViewModel.cs
index bbf03ea..a9e2dbb 100644
--- a/src/Stampeded/Documents/StartDocumentViewModel.cs
+++ b/src/Stampeded/Documents/StartDocumentViewModel.cs
@@ -160,6 +160,15 @@ public sealed partial class StartState : ObservableObject
[ObservableProperty]
bool isPreparing;
+ /// Why the review that was opening did not open, or empty. While it is set the
+ /// overlay shows this instead of the checklist and stays until it is dismissed: an overlay
+ /// that simply went away would look like an open that was never asked for.
+ [ObservableProperty]
+ [NotifyPropertyChangedFor(nameof(PrepareFailed))]
+ string prepareError = "";
+
+ public bool PrepareFailed => PrepareError.Length > 0;
+
[ObservableProperty]
string recentFilter = "";
@@ -182,6 +191,9 @@ public class StartDocumentViewModel : Document
/// tooltips that name the host.
public string HostName => workspace.HostName;
bool openOverviewWhenReady;
+ // Counts the opens started from here, so one that ends after a later one began can tell
+ // the overlay is no longer its to change.
+ int openAttempt;
public StartState State { get; } = new();
public ObservableCollection Recents { get; } = [];
@@ -505,14 +517,10 @@ async Task RefreshSyncStatesAsync(Dictionary prsByBranch)
public void OpenPr(PrSummary pr) => OpenPrNumber(pr.Number);
public void OpenPrNumber(int number)
- {
- BeginPreparation();
- workspace.OpenPrAsync(number).HandleExceptions();
- }
+ => OpenWithPreparation($"PR #{number}", () => workspace.OpenPrAsync(number));
public void OpenBranch(BranchRow row)
{
- BeginPreparation();
// A stash reviews as the range from the commit it was taken on to the stash
// commit itself, which is exactly the stashed change.
var (baseRef, head) = row.IsStash
@@ -522,7 +530,8 @@ public void OpenBranch(BranchRow row)
// along, read against the local commits. Opening the same change from the pull request
// list reads the pushed head instead - which is the difference between the two lists
// once a branch has moved on from what was pushed.
- workspace.OpenLocalRangeAsync(baseRef, head, row.PrNumber).HandleExceptions();
+ OpenWithPreparation(row.IsStash ? "the stash" : head,
+ () => workspace.OpenLocalRangeAsync(baseRef, head, row.PrNumber));
}
/// Gives a stash a durable name by pointing a new branch at its commit. The
@@ -969,10 +978,60 @@ public void OpenBranchPrOnHost(BranchRow row)
void BeginPreparation()
{
openOverviewWhenReady = true;
+ State.PrepareError = "";
State.IsPreparing = true;
UpdatePreparation();
}
+ ///
+ /// Opens a review behind the preparation overlay, and owns what the overlay does when the
+ /// open fails: it says why and waits to be dismissed. The overlay covers the window and
+ /// only a finished open takes it down, so a failure that went to the log alone left the
+ /// reader behind a spinner that would never stop.
+ ///
+ void OpenWithPreparation(string what, Func open)
+ {
+ int attempt = ++openAttempt;
+ BeginPreparation();
+ RunAsync().HandleExceptions();
+
+ async Task RunAsync()
+ {
+ try
+ {
+ await open();
+ }
+ catch (OperationCanceledException)
+ {
+ // Cancelled by a later open, whose overlay this now is, or by the workspace
+ // going away. Only in the second case is there an overlay left to take down.
+ if (attempt == openAttempt)
+ State.IsPreparing = false;
+ }
+ catch (Exception ex)
+ {
+ CliLog.Write("error", $"open of {what} failed: {ex.Message}");
+ if (attempt != openAttempt)
+ return;
+ openOverviewWhenReady = false;
+ string reason = ex is ToolFailedException failure ? ExternalTool.Explain(failure) : ex.Message;
+ State.PrepareError = $"Could not open {what}: {reason}";
+ State.Status = State.PrepareError;
+ // The error is drawn inside the preparation overlay, which is visible only while
+ // this is true. Usually it still is; "Continue now" pressed before the failure
+ // arrived has cleared it, and the reason would be set with nothing showing it.
+ State.IsPreparing = true;
+ }
+ }
+ }
+
+ /// Takes down the overlay a failed open left its reason on.
+ public void DismissPrepareError()
+ {
+ State.PrepareError = "";
+ State.IsPreparing = false;
+ }
+
public void ContinueNow()
{
openOverviewWhenReady = false;
diff --git a/src/Stampeded/MainWindow.axaml b/src/Stampeded/MainWindow.axaml
index 8785497..4297aa9 100644
--- a/src/Stampeded/MainWindow.axaml
+++ b/src/Stampeded/MainWindow.axaml
@@ -229,7 +229,17 @@
-
+
+
+
+
+
+
+
+
+
@@ -253,6 +263,7 @@
+
diff --git a/src/Stampeded/MainWindow.axaml.cs b/src/Stampeded/MainWindow.axaml.cs
index 184c0d4..75c26d0 100644
--- a/src/Stampeded/MainWindow.axaml.cs
+++ b/src/Stampeded/MainWindow.axaml.cs
@@ -391,6 +391,7 @@ static void UsePassBaseline(PassBaselineKind kind)
void OnOpenOverview(object? s, EventArgs e) => App.Workspace?.OpenOverview();
void OnContinueFromPrepare(object? s, RoutedEventArgs e) => App.Workspace?.StartPage?.ContinueNow();
+ void OnDismissPrepareError(object? s, RoutedEventArgs e) => App.Workspace?.StartPage?.DismissPrepareError();
void OnOpenOnHost(object? s, EventArgs e)
{
diff --git a/src/Stampeded/ReviewWorkspace.cs b/src/Stampeded/ReviewWorkspace.cs
index 7870b6a..943e9aa 100644
--- a/src/Stampeded/ReviewWorkspace.cs
+++ b/src/Stampeded/ReviewWorkspace.cs
@@ -399,14 +399,17 @@ public async Task OpenLocalRangeAsync(string baseRef, string headRef, int? prNum
}
string headSha = await ResolveAsync(headRef, ct);
string baseSha = await Git.GetMergeBaseAsync(await ResolveAsync(baseRef, ct), headSha, ct);
- DirtyWorktreePath = await FindDirtyCheckoutAsync(headRef, ct);
+ // Nothing of the review on screen is touched until the change has been read, so an open
+ // that fails up to here leaves that review exactly as it was.
+ string? dirty = await FindDirtyCheckoutAsync(headRef, ct);
var committed = await Git.DiffAsync(baseSha, headSha, ct);
- var files = DirtyWorktreePath is { } dirty
+ var files = dirty is not null
? await Git.DiffWorkingTreeAsync(dirty, baseSha, ct)
: committed;
- UncommittedFileCount = Math.Max(0, files.Count - committed.Count);
ct.ThrowIfCancellationRequested();
+ DirtyWorktreePath = dirty;
+ UncommittedFileCount = Math.Max(0, files.Count - committed.Count);
Scopes.Reset();
Reviewers = null;
CurrentPr = detail;
@@ -422,9 +425,17 @@ public async Task OpenLocalRangeAsync(string baseRef, string headRef, int? prNum
Files = files;
changed = ChangedLines.From(files);
ReviewReset?.Invoke();
- Store.OpenLocal(Path.GetFileName(RepoPath), $"{baseRef}..{headRef}", headSha, baseSha);
- await ApplyReReviewCarryOverAsync(ct);
- await PinReviewHeadsAsync(ct);
+ try
+ {
+ Store.OpenLocal(Path.GetFileName(RepoPath), $"{baseRef}..{headRef}", headSha, baseSha);
+ await ApplyReReviewCarryOverAsync(ct);
+ await PinReviewHeadsAsync(ct);
+ }
+ catch (Exception ex) when (ex is not OperationCanceledException)
+ {
+ AbandonHalfOpenedReview();
+ throw;
+ }
ComputeChurnAsync().HandleExceptions();
history.Clear();
CloseDocumentsExceptStart();
@@ -459,9 +470,9 @@ public async Task OpenPrAsync(int number)
CliLog.Write("action", $"open PR #{number}");
PrDetail detail;
string headSha, baseSha;
- Offline = false;
- OfflineSince = null;
- snapshot = null;
+ // Held here until the change has been read: an open that fails before that leaves the
+ // review that was on screen exactly as it was, including what it says about being offline.
+ PrSnapshot? openedFrom = null;
try
{
detail = await Host.GetPrAsync(number, ct);
@@ -480,15 +491,16 @@ public async Task OpenPrAsync(int number)
detail = cached.Detail;
headSha = cached.HeadSha;
baseSha = cached.BaseSha;
- Offline = true;
- OfflineSince = cached.TakenAt;
- snapshot = cached;
+ openedFrom = cached;
}
- DirtyWorktreePath = null;
- UncommittedFileCount = 0;
var files = await Git.DiffAsync(baseSha, headSha, ct);
ct.ThrowIfCancellationRequested();
+ DirtyWorktreePath = null;
+ UncommittedFileCount = 0;
+ Offline = openedFrom is not null;
+ OfflineSince = openedFrom?.TakenAt;
+ snapshot = openedFrom;
Scopes.Reset();
Reviewers = null;
CurrentPr = detail;
@@ -499,9 +511,17 @@ public async Task OpenPrAsync(int number)
Files = files;
changed = ChangedLines.From(files);
ReviewReset?.Invoke();
- Store.Open(Path.GetFileName(RepoPath), number, headSha, baseSha);
- await ApplyReReviewCarryOverAsync(ct);
- await PinReviewHeadsAsync(ct);
+ try
+ {
+ Store.Open(Path.GetFileName(RepoPath), number, headSha, baseSha);
+ await ApplyReReviewCarryOverAsync(ct);
+ await PinReviewHeadsAsync(ct);
+ }
+ catch (Exception ex) when (ex is not OperationCanceledException)
+ {
+ AbandonHalfOpenedReview();
+ throw;
+ }
ComputeChurnAsync().HandleExceptions();
history.Clear();
CloseDocumentsExceptStart();
@@ -1654,6 +1674,18 @@ public void CloseReview()
CliLog.Write("action", "review closed");
}
+ ///
+ /// The way out of an open that failed after the review's state was already replaced. The
+ /// review that was there is gone and the new one is not whole - commits named, nothing
+ /// loaded behind them - so neither can be left on screen. Closing is the one state that
+ /// describes what the reader has: no review, and the start page to pick one from.
+ ///
+ void AbandonHalfOpenedReview()
+ {
+ CliLog.Write("action", "open failed part-way; closing what it left behind");
+ CloseReview();
+ }
+
/// What was open before a scope switch or a reload rebuilt the review, so it can
/// be put back.
readonly record struct OpenDocuments(IReadOnlyList Ids, string? Active);