diff --git a/README.md b/README.md index 6943c5a..d5807ab 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ On launch, gclo shows a brief branded splash, then the **Quick Sync** connect ca Finished sync: every row Done, with the summary 58 cloned, 0 updated, 1 failed, 0 canceled of 59

-8. **Cancel** at any time: in-flight repos stop cleanly, unstarted repos are marked Canceled, and the summary still appears. After a run, the result bar offers **Retry failed** when something failed (it re-runs exactly the failed rows), otherwise **Open folder**. The folder button in the toolbar opens the target folder in Explorer whenever no sync is running. +8. **Cancel** at any time: in-flight repos stop cleanly, unstarted repos are marked Canceled, and the summary still appears. After a run, the result bar offers **Retry failed** when something failed (it re-runs exactly the failed rows and leaves your selection alone), otherwise **Open folder**. The folder button in the toolbar opens the target folder in Explorer whenever no sync is running. **When something goes wrong:** failures are isolated per repository — one broken repo never stops the rest, and a failed row shows its error message inline. Failed clones are cleaned up, and a clone whose checkout never finished (a crash, a kill, a cleanup that could not delete the folder) is flagged inside its `.git`, so a partial working tree is never mistaken for an up-to-date repo on the next run — gclo completes the checkout instead. What it completes is its own. On Windows, a file that is already where the repository has one is replaced only when it is what the unfinished checkout wrote there (its content, or the beginning of it, line endings aside); for anything else the row fails and names the file: move it and sync again. The same check guards a repository that was empty when it was cloned and gets its first files later. Such a clone follows the origin's default branch when nothing has been put into its folder (otherwise it keeps the branch its HEAD names, and stays empty until origin has that branch); a clone that has branches of its own is left as it is when its HEAD names a branch without commits (a branch you started there yourself). If you have worked in an unfinished clone with git in the meantime, gclo finishes it only when every file there is still the repository's own and nothing you added with `git add` would be removed; otherwise the row fails and names the file. While a canceled clone or update is still ending, syncing that repository again fails with a message to try again when it has finished. Repositories with Windows-impossible paths are the deliberate exception: gclo checks every path in the incoming tree against Windows file-system rules *before* checkout — invalid characters, reserved device names like `CON`, trailing spaces or dots, names that differ only by case — and when it finds any, the repo is marked Failed with a per-path list of reasons, but the fully fetched `.git` is **kept** (nothing was ever checked out). Recovery happens in place, with no re-download: click the row's **Resolve…** link to rename each offending path (safe names are pre-suggested) or skip it, and gclo materializes the working tree right there; the CLI does the same automatically with [`--sanitize-paths`](docs/CLI.md#windows-invalid-paths-and---sanitize-paths). This applies to a repository gclo could not check out when it cloned it. A repository that was already checked out normally is never rewritten: when an update brings it such a path, it fails and nothing in it is changed, and **Resolve…** (or `--sanitize-paths`) then says how to get a fresh clone with the renames instead of touching it. When WSL is installed with git in its default distribution, the same dialog offers **Clone in WSL instead**: the repository is cloned unchanged into `~/gclo//` inside WSL (reachable from Explorer as `\wsl.localhost\\home\...`), the token travels only through the environment, and the Windows copy is left as it is. Long paths beyond the classic 260-character limit are handled automatically (`core.longpaths`). @@ -124,7 +124,7 @@ On launch, gclo shows a brief branded splash, then the **Quick Sync** connect ca ### Saved accounts -Quick Sync is ad-hoc — nothing is saved. For organizations you sync regularly, **Add account** stores a reusable profile: a name, the organization, target folder, and parallelism, with the token kept in the Windows Credential Manager (never in a file). Each account gets its own entry in the navigation pane and its own workspace; **Sync all** runs every saved account in turn. A status bar above the workspace is there for the whole pass, whichever workspace is on screen: it says which account is syncing, which of how many, and how far along it is, takes you to that account, and holds the result when the pass is over. The pane shows each account's live status as well. **Cancel sync all** stops the pass after the account that is syncing. +Quick Sync is ad-hoc — nothing is saved. For organizations you sync regularly, **Add account** stores a reusable profile: a name, the organization, target folder, and parallelism, with the token kept in the Windows Credential Manager (never in a file). Each account gets its own entry in the navigation pane and its own workspace; **Sync all** runs every saved account in turn. A status bar above the workspace is there for the whole pass, whichever workspace is on screen: it says which account is syncing, which of how many, and how far along it is, takes you to that account, and holds the result when the pass is over. The pane shows each account's live status as well. **Cancel sync all** stops the pass after the account that is syncing, and the result says how many accounts were not started. An account that could not be synced (it has no token, or its repositories could not be loaded) is marked in the pane and counted as needing attention, so a pass in which it was left out does not read as a success. An account's token is changed with **Edit account…** on the account's page (**Edit…** in the pane's menu); the token box on an account's page takes no input. Sync all syncs the repositories each account's page lists: **Refresh** on that page picks up repositories created since it was loaded.

Add account wizard, step 1 of 4: account name and optional description diff --git a/gclo.Engine.Tests/SyncAllCoordinatorTests.cs b/gclo.Engine.Tests/SyncAllCoordinatorTests.cs index 447be17..ceef39f 100644 --- a/gclo.Engine.Tests/SyncAllCoordinatorTests.cs +++ b/gclo.Engine.Tests/SyncAllCoordinatorTests.cs @@ -178,7 +178,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable [Fact] - public async Task RunAsync_DefaultTokenAccount_WithoutADefaultToken_IsSkipped_NotRun() + public async Task RunAsync_DefaultTokenAccount_WithoutADefaultToken_NeedsAttention_AndIsNotRun() { WorkspaceViewModel shared = CreateDefaultTokenWorkspace("shared", withDefaultToken: false); Assert.True(shared.UsesDefaultToken); @@ -188,10 +188,9 @@ public sealed class SyncAllCoordinatorTests : IDisposable SyncAllResult result = await _coordinator.RunAsync([shared], CancellationToken.None); - Assert.Equal(0, result.Ran); - Assert.Equal(1, result.Skipped); - Assert.Equal([SyncAllAccountState.Queued, SyncAllAccountState.Skipped], states); - Assert.Empty(_lister.Organizations); // never asked GitHub: skipped before loading + Assert.Equal(new SyncAllResult(Ran: 0, Skipped: 0, WasCanceled: false, Failed: 1, CouldNotSync: 1), result); + Assert.Equal([SyncAllAccountState.Queued, SyncAllAccountState.Failed], states); // not a success with one skipped (#182) + Assert.Empty(_lister.Organizations); // never asked GitHub: stopped before loading } @@ -298,7 +297,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable [Fact] - public async Task RunAsync_SkipsAWorkspaceWhoseLoadFails_AndMovesOn() + public async Task RunAsync_AWorkspaceWhoseLoadFails_NeedsAttention_AndThePassMovesOn() { _lister.Handler = org => string.Equals(org, "org-alpha" , StringComparison.Ordinal) ? throw new InvalidOperationException("org not found") @@ -309,7 +308,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable SyncAllResult result = await _coordinator.RunAsync(new[] { alpha, beta }, CancellationToken.None); - Assert.Equal(new SyncAllResult(Ran: 1, Skipped: 1, WasCanceled: false), result); + Assert.Equal(new SyncAllResult(Ran: 1, Skipped: 0, WasCanceled: false, Failed: 1, CouldNotSync: 1), result); Assert.False(alpha.HasLoadedRepos); Assert.Equal(new[] { "org-beta-repo" }, git.ClonedRepoNames); } @@ -354,7 +353,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable gate.SetResult(); // ...which must still run to completion SyncAllResult result = await run.WaitAsync(TimeSpan.FromSeconds(30)); - Assert.Equal(new SyncAllResult(Ran: 1, Skipped: 0, WasCanceled: true), result); + Assert.Equal(new SyncAllResult(Ran: 1, Skipped: 0, WasCanceled: true, NotStarted: 1), result); Assert.Equal(SyncStatus.Done, alpha.Repos[0].Status); // finished, not canceled Assert.False(alpha.IsRunning); Assert.False(beta.HasLoadedRepos); // the second account never started @@ -374,7 +373,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable SyncAllResult result = await _coordinator.RunAsync(new[] { workspace }, cts.Token); - Assert.Equal(new SyncAllResult(Ran: 0, Skipped: 0, WasCanceled: true), result); + Assert.Equal(new SyncAllResult(Ran: 0, Skipped: 0, WasCanceled: true, NotStarted: 1), result); Assert.False(workspace.HasLoadedRepos); Assert.Empty(git.CloneCalls); } @@ -497,15 +496,14 @@ public sealed class SyncAllCoordinatorTests : IDisposable [Fact] - public async Task RunAsync_DefaultTokenAccount_WhoseTokenWasRemovedAfterItOpened_IsSkipped() + public async Task RunAsync_DefaultTokenAccount_WhoseTokenWasRemovedAfterItOpened_NeedsAttention() { WorkspaceViewModel shared = CreateDefaultTokenWorkspace("shared", withDefaultToken: true); _vault.Delete(AppSettings.DefaultTokenVaultId); // removed in Settings meanwhile SyncAllResult result = await _coordinator.RunAsync([shared], CancellationToken.None); - Assert.Equal(0, result.Ran); - Assert.Equal(1, result.Skipped); + Assert.Equal(new SyncAllResult(Ran: 0, Skipped: 0, WasCanceled: false, Failed: 1, CouldNotSync: 1), result); Assert.Empty(_lister.Organizations); // GitHub was never asked with the old token } @@ -514,7 +512,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable // ---------------------------------------------------------------- review of #157: no success for a sync that never ran [Fact] - public async Task RunAsync_AnAccountWhoseOwnTokenIsGone_IsSkipped_NotReportedAsSucceeded() + public async Task RunAsync_AnAccountWhoseOwnTokenIsGone_NeedsAttention_NotReportedAsSucceeded() { WorkspaceViewModel workspace = CreateWorkspace("own"); await workspace.LoadReposCommand.ExecuteAsync(null); // a loaded table: the sync command can run without a token @@ -524,9 +522,8 @@ public sealed class SyncAllCoordinatorTests : IDisposable SyncAllResult result = await _coordinator.RunAsync([workspace], CancellationToken.None); - Assert.Equal(0, result.Ran); - Assert.Equal(1, result.Skipped); - Assert.Equal([SyncAllAccountState.Queued, SyncAllAccountState.Skipped], states); + Assert.Equal(new SyncAllResult(Ran: 0, Skipped: 0, WasCanceled: false, Failed: 1, CouldNotSync: 1), result); + Assert.Equal([SyncAllAccountState.Queued, SyncAllAccountState.Failed], states); } @@ -544,8 +541,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable SyncAllResult result = await _coordinator.RunAsync([workspace], CancellationToken.None); Assert.Equal(RunResultKind.Error, workspace.ResultKind); - Assert.Equal(0, result.Ran); - Assert.Equal(1, result.Skipped); + Assert.Equal(new SyncAllResult(Ran: 0, Skipped: 0, WasCanceled: false, Failed: 1, CouldNotSync: 1), result); Assert.Equal([SyncAllAccountState.Queued, SyncAllAccountState.Running, SyncAllAccountState.Failed], states); } @@ -610,7 +606,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable Assert.True(status.IsOpen); // the result stays until it is closed Assert.Equal(SyncAllOutcome.Finished, status.Outcome); Assert.Equal("Sync all finished", status.Title); - Assert.Equal("2 accounts synced, 0 skipped.", status.Message); + Assert.Equal("2 of 2 accounts synced.", status.Message); Assert.Null(status.CurrentAccountId); Assert.Null(status.Fraction); @@ -630,14 +626,14 @@ public sealed class SyncAllCoordinatorTests : IDisposable "beta: 0 of 1 repository.", "beta: 1 of 1 repository.", "beta: starting.", - "2 accounts synced, 0 skipped.", + "2 of 2 accounts synced.", ], said); // The pass is over: what the accounts do next is no longer the bar's business. status.IsOpen = false; await alpha.SyncCommand.ExecuteAsync(null); - Assert.Equal("2 accounts synced, 0 skipped.", status.Message); + Assert.Equal("2 of 2 accounts synced.", status.Message); Assert.False(status.IsOpen); } @@ -669,7 +665,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable await run.WaitAsync(TimeSpan.FromSeconds(30)); Assert.Contains(0.5, fractions); - Assert.Equal("1 account synced, 0 skipped.", status.Message); + Assert.Equal("1 of 1 account synced.", status.Message); } @@ -701,7 +697,7 @@ public sealed class SyncAllCoordinatorTests : IDisposable Assert.True(result.WasCanceled); Assert.Equal(SyncAllOutcome.Canceled, status.Outcome); Assert.Equal("Sync all canceled", status.Title); - Assert.Equal("1 account synced, 0 skipped.", status.Message); + Assert.Equal("1 of 2 accounts synced. 1 account was not started.", status.Message); Assert.False(status.IsRunning); // The next pass starts clean. @@ -727,10 +723,10 @@ public sealed class SyncAllCoordinatorTests : IDisposable SyncAllResult result = await _coordinator.RunAsync([failing, blocked, fine], CancellationToken.None); - Assert.Equal(new SyncAllResult(Ran: 2, Skipped: 1, WasCanceled: false, Failed: 2), result); + Assert.Equal(new SyncAllResult(Ran: 2, Skipped: 0, WasCanceled: false, Failed: 2, CouldNotSync: 1), result); Assert.Equal(SyncAllOutcome.FinishedWithFailures, status.Outcome); Assert.Equal("Sync all finished", status.Title); - Assert.Equal("2 accounts synced, 1 skipped. 2 accounts need attention; they are marked in the pane.", status.Message); + Assert.Equal("2 of 3 accounts synced. 2 accounts need attention; they are marked in the pane.", status.Message); } @@ -752,8 +748,13 @@ public sealed class SyncAllCoordinatorTests : IDisposable SyncAllResult result = await _coordinator.RunAsync([failing, never], cts.Token); - Assert.Equal(new SyncAllResult(Ran: 1, Skipped: 0, WasCanceled: true, Failed: 1), result); - Assert.Equal("1 account synced, 0 skipped. 1 account needs attention; it is marked in the pane.", SyncAllStatus.Describe(result)); + Assert.Equal(new SyncAllResult(Ran: 1, Skipped: 0, WasCanceled: true, Failed: 1, NotStarted: 1), result); + Assert.Equal( + "1 of 2 accounts synced. 1 account needs attention; it is marked in the pane. 1 account was not started.", + SyncAllStatus.Describe(result)); + Assert.Equal( + "0 of 3 accounts synced. 3 accounts were not started.", + SyncAllStatus.Describe(new SyncAllResult(Ran: 0, Skipped: 0, WasCanceled: true, NotStarted: 3))); } @@ -833,4 +834,238 @@ public sealed class SyncAllCoordinatorTests : IDisposable Assert.Throws(() => status.Enter(1, null!)); Assert.Throws(() => status.End(null!)); } + + + + // ---------------------------------------------------------------- release reviews of 1.1.1 (#182) + + [Fact] + public async Task AnAccountThatCouldNotBeSynced_DoesNotLeaveThePassLookingLikeASuccess() + { + // Its token is gone: nothing else is wrong in the pass. + WorkspaceViewModel shared = CreateDefaultTokenWorkspace("shared", withDefaultToken: false); + var status = new SyncAllStatus(); + _coordinator.Status = status; + + await _coordinator.RunAsync([shared], CancellationToken.None); + + Assert.Equal(SyncAllOutcome.FinishedWithFailures, status.Outcome); + Assert.Equal("0 of 1 account synced. 1 account needs attention; it is marked in the pane.", status.Message); + } + + + + [Theory] + [InlineData(false)] + [InlineData(true)] // a repository had failed before the cancel: the account needs attention + public async Task AnAccountWhoseOwnSyncWasCanceled_IsNotCountedAsSynced_AndThePassGoesOn(bool afterAFailure) + { + _lister.Handler = _ => [Repo("one"), Repo("two"), Repo("three")]; + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var git = new FakeGitClient + { + CloneHandler = async (_, path, _, _, ct) => + { + if(afterAFailure && path.EndsWith("one", StringComparison.Ordinal)) + { + throw new InvalidOperationException("boom"); // one at a time, in order: this one is over before the next begins + } + entered.TrySetResult(); + await Task.Delay(Timeout.Infinite, ct); + }, + }; + WorkspaceViewModel alpha = CreateWorkspace("alpha", git); + alpha.MaxConcurrency = 1; + WorkspaceViewModel beta = CreateWorkspace("beta"); + var states = new List(); + _coordinator.AccountStateChanged = (id, state) => + { + if(id == alpha.AccountId) + { + states.Add(state); + } + }; + var status = new SyncAllStatus(); + _coordinator.Status = status; + + Task run = _coordinator.RunAsync([alpha, beta], CancellationToken.None); + await entered.Task.WaitAsync(TimeSpan.FromSeconds(30)); + alpha.SyncCancelCommand.Execute(null); // Cancel in that account's own workspace + SyncAllResult result = await run.WaitAsync(TimeSpan.FromSeconds(30)); + + Assert.Equal(RunResultKind.Canceled, alpha.ResultKind); + Assert.Equal( + afterAFailure + ? new SyncAllResult(Ran: 1, Skipped: 0, WasCanceled: false, Failed: 1, CouldNotSync: 1) + : new SyncAllResult(Ran: 1, Skipped: 1, WasCanceled: false), + result); // beta ran + Assert.Equal( + [SyncAllAccountState.Queued, SyncAllAccountState.Running, afterAFailure ? SyncAllAccountState.Failed : SyncAllAccountState.Skipped], + states); + Assert.Equal( + afterAFailure + ? "1 of 2 accounts synced. 1 account needs attention; it is marked in the pane." + : "1 of 2 accounts synced. 1 account was skipped (busy, canceled, or nothing selected).", + status.Message); + } + + + + [Theory] + [InlineData(true)] + [InlineData(false)] // one repository was not selected: it is not selected afterwards either + public async Task RetryFailed_LeavesTheSelectionAlone_SoTheNextSyncCoversWhatItCoveredBefore(bool allSelected) + { + _lister.Handler = _ => [Repo("one"), Repo("two"), Repo("three")]; + bool failTwo = true; + var git = new FakeGitClient + { + CloneHandler = (_, path, _, _, _) => failTwo && path.EndsWith("two", StringComparison.Ordinal) + ? Task.FromException(new InvalidOperationException("boom")) + : Task.CompletedTask, + }; + WorkspaceViewModel alpha = CreateWorkspace("alpha", git); + await alpha.LoadReposCommand.ExecuteAsync(null); + if(!allSelected) + { + alpha.Repos.Single(repo => string.Equals(repo.Name, "three", StringComparison.Ordinal)).IsSelected = false; + } + int selected = allSelected ? 3 : 2; + await alpha.SyncCommand.ExecuteAsync(null); + Assert.True(alpha.HasFailedRepos); + + failTwo = false; + await alpha.RetryFailedCommand.ExecuteAsync(null); + + Assert.False(alpha.HasFailedRepos); + Assert.Equal(selected, alpha.SelectedCount); // the retry ran one repository; the selection is still the user's + Assert.Equal(selected, alpha.Repos.Count(repo => repo.IsSelected)); + Assert.Equal(allSelected, alpha.Repos.Single(repo => string.Equals(repo.Name, "three", StringComparison.Ordinal)).IsSelected); + + int before = git.CloneCalls.Count; + SyncAllResult result = await _coordinator.RunAsync([alpha], CancellationToken.None); + + Assert.Equal(new SyncAllResult(Ran: 1, Skipped: 0, WasCanceled: false), result); + Assert.Equal(selected, git.CloneCalls.Count - before); // not only the one that had failed + } + + + + [Fact] + public void AnAccountsPage_IsToldWhenItsTokenIsNoLongerMissing() + { + WorkspaceViewModel shared = CreateDefaultTokenWorkspace("shared", withDefaultToken: false); + var changed = new List(); + shared.PropertyChanged += (_, e) => changed.Add(e.PropertyName); + Assert.NotNull(shared.MissingTokenDescription); + + _vault.Store(AppSettings.DefaultTokenVaultId, "tok-default"); // saved in Settings meanwhile + shared.RefreshAccountToken(); + + Assert.Null(shared.MissingTokenDescription); + Assert.Null(shared.MissingTokenAdvice); + Assert.Contains(changed, name => string.Equals(name, nameof(WorkspaceViewModel.MissingTokenDescription), StringComparison.Ordinal)); + Assert.Contains(changed, name => string.Equals(name, nameof(WorkspaceViewModel.MissingTokenAdvice), StringComparison.Ordinal)); + } + + + + [Fact] + public void AnAccountsPage_SaysWhereItsTokenIsSaved_InTheWordsOfTheWindow() + { + WorkspaceViewModel shared = CreateDefaultTokenWorkspace("shared", withDefaultToken: false); + WorkspaceViewModel own = CreateWorkspace("own"); + _vault.Delete(own.AccountId!.Value); + own.RefreshAccountToken(); + + Assert.Equal( + "No default token is saved. Save one in File > Settings, or give this account its own token with Edit account…", + shared.MissingTokenAdvice); + Assert.Equal("No token is saved for this account. Choose Edit account… and enter one.", own.MissingTokenAdvice); + Assert.Null(CreateWorkspace("fine").MissingTokenAdvice); + } + + + + [Fact] + public async Task SeveralAccountsThatWereSkipped_AreCountedOnce_AndNotAsNeedingAttention() + { + WorkspaceViewModel first = CreateWorkspace("first"); + WorkspaceViewModel second = CreateWorkspace("second"); + _lister.Handler = _ => []; // nothing to sync in either + var status = new SyncAllStatus(); + _coordinator.Status = status; + + SyncAllResult result = await _coordinator.RunAsync([first, second], CancellationToken.None); + + Assert.Equal(new SyncAllResult(Ran: 0, Skipped: 2, WasCanceled: false), result); + Assert.Equal(2, result.Total); + Assert.Equal(SyncAllOutcome.Finished, status.Outcome); + Assert.Equal("0 of 2 accounts synced. 2 accounts were skipped (busy, canceled, or nothing selected).", status.Message); + } + + + + [Fact] + public async Task OneRepository_IsCountedInTheSingular() + { + (FakeGitClient git, Task entered, TaskCompletionSource gate) = GatedGit(); + WorkspaceViewModel alpha = CreateWorkspace("alpha", git); + + await alpha.LoadReposCommand.ExecuteAsync(null); + Assert.Equal("1 repository loaded.", alpha.StatusText); + + Task sync = alpha.SyncCommand.ExecuteAsync(null); + await entered.WaitAsync(TimeSpan.FromSeconds(30)); + Assert.Equal("Syncing 1 repository...", alpha.StatusText); + gate.SetResult(); + await sync.WaitAsync(TimeSpan.FromSeconds(30)); + } + + + + [Fact] + public async Task RetryFailed_RunsTheFailedRows_AlsoWhenTheyAreNotSelected_AndASelectionChangeMadeMeanwhileStays() + { + _lister.Handler = _ => [Repo("one"), Repo("two"), Repo("three")]; + bool failTwo = true; + var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var git = new FakeGitClient + { + CloneHandler = (_, path, _, _, _) => + { + if(!path.EndsWith("two", StringComparison.Ordinal)) + { + return Task.CompletedTask; + } + if(failTwo) + { + return Task.FromException(new InvalidOperationException("boom")); + } + entered.TrySetResult(); + return gate.Task; + }, + }; + WorkspaceViewModel alpha = CreateWorkspace("alpha", git); + await alpha.LoadReposCommand.ExecuteAsync(null); + await alpha.SyncCommand.ExecuteAsync(null); + RepoItemViewModel Row(string name) => alpha.Repos.Single(repo => string.Equals(repo.Name, name, StringComparison.Ordinal)); + Row("two").IsSelected = false; // the failed row is taken out of the selection: the retry is for it all the same + int before = git.CloneCalls.Count; + + failTwo = false; + Task retry = alpha.RetryFailedCommand.ExecuteAsync(null); + await entered.Task.WaitAsync(TimeSpan.FromSeconds(30)); + Row("one").IsSelected = false; // the checkboxes stay enabled while a run is going on + gate.SetResult(); + await retry.WaitAsync(TimeSpan.FromSeconds(30)); + + Assert.Equal(SyncStatus.Done, Row("two").Status); + Assert.Equal(1, git.CloneCalls.Count - before); // the failed row, and only it + Assert.False(Row("one").IsSelected); + Assert.False(Row("two").IsSelected); + Assert.True(Row("three").IsSelected); + Assert.Equal(1, alpha.SelectedCount); + } } diff --git a/gclo.Engine.Tests/WorkspaceViewModelTests.cs b/gclo.Engine.Tests/WorkspaceViewModelTests.cs index 4d9b682..e46876c 100644 --- a/gclo.Engine.Tests/WorkspaceViewModelTests.cs +++ b/gclo.Engine.Tests/WorkspaceViewModelTests.cs @@ -641,9 +641,9 @@ public sealed class WorkspaceViewModelTests : IDisposable Assert.Equal(SyncStatus.Done, byName["bravo"].Status); Assert.Null(byName["bravo"].Error); Assert.Equal(SyncStatus.Done, byName["alpha"].Status); // untouched by the retry - Assert.True(byName["bravo"].IsSelected); // retry selected exactly the failures - Assert.False(byName["alpha"].IsSelected); - Assert.False(byName["charlie"].IsSelected); + Assert.True(byName["bravo"].IsSelected); // the retry ran exactly the failure, and the selection was not touched (#182) + Assert.True(byName["alpha"].IsSelected); + Assert.True(byName["charlie"].IsSelected); Assert.Equal(1, vm.TotalCount); // run-scoped (#22): the retry run was just bravo Assert.Equal(1, vm.CompletedCount); Assert.False(vm.RetryFailedCommand.CanExecute(null)); // nothing failed anymore diff --git a/gclo.ViewModels/SyncAllAccountState.cs b/gclo.ViewModels/SyncAllAccountState.cs index e6809d0..33234fe 100644 --- a/gclo.ViewModels/SyncAllAccountState.cs +++ b/gclo.ViewModels/SyncAllAccountState.cs @@ -18,9 +18,16 @@ public enum SyncAllAccountState ///

Finished with no failed repositories. Succeeded, - /// Finished, but at least one repository failed. + /// + /// Needs attention: its sync finished with at least one failed repository, or it + /// could not be synced (no token, repositories that could not be loaded, a sync + /// that could not start). + /// Failed, - /// Not run: busy, unloadable, nothing to sync, or the queue was canceled. + /// + /// Not synced, and nothing is wrong with it: busy, nothing to sync, its own sync + /// was canceled, or the queue was canceled before its turn. + /// Skipped, } diff --git a/gclo.ViewModels/SyncAllCoordinator.cs b/gclo.ViewModels/SyncAllCoordinator.cs index 51ccdf3..b6d96af 100644 --- a/gclo.ViewModels/SyncAllCoordinator.cs +++ b/gclo.ViewModels/SyncAllCoordinator.cs @@ -40,9 +40,12 @@ public sealed class SyncAllCoordinator(IActivityLog log) /// /// Syncs each workspace of in list order, /// loading its repositories first when needed. A workspace is skipped (counted, - /// logged, never fatal) when it is already busy, when its repositories cannot be - /// loaded, or when its sync command cannot run. Canceling - /// stops the queue after the in-flight account rather than canceling its sync. + /// logged, never fatal) when it is already busy or has nothing to sync. One that + /// cannot be synced (it has no token, its repositories cannot be loaded, its sync + /// cannot start) is left out too, and counted and announced as failed, not as + /// skipped: the pass must not read as a success when an account was not synced + /// (#182). Canceling stops the queue after the in-flight + /// account rather than canceling its sync. /// public async Task RunAsync( IReadOnlyList accountWorkspaces, CancellationToken ct) @@ -80,6 +83,7 @@ public sealed class SyncAllCoordinator(IActivityLog log) int ran = 0; int skipped = 0; int failed = 0; + int couldNotSync = 0; for(int i = 0; i < accountWorkspaces.Count; i++) { @@ -95,7 +99,7 @@ public sealed class SyncAllCoordinator(IActivityLog log) Announce(accountWorkspaces[rest], SyncAllAccountState.Skipped); } _log.Info("Sync all: canceled — remaining accounts were not started."); - return new SyncAllResult(ran, skipped, WasCanceled: true, failed); + return new SyncAllResult(ran, skipped, WasCanceled: true, failed, NotStarted: accountWorkspaces.Count - i, couldNotSync); } if(workspace.IsRunning || workspace.IsLoadingRepos || workspace.IsResolvingPaths) @@ -113,9 +117,10 @@ public sealed class SyncAllCoordinator(IActivityLog log) // Whichever source the account's token comes from, it holds none: say // which, instead of a sync that cannot start or a generic auth failure // that sends the user to the wrong place. - skipped++; - Announce(workspace, SyncAllAccountState.Skipped); - _log.Info($"Sync all: skipped '{workspace.DisplayName}' — {missingToken}"); + couldNotSync++; + failed++; + Announce(workspace, SyncAllAccountState.Failed); + _log.Info($"Sync all: '{workspace.DisplayName}' could not be synced — {missingToken}"); continue; } @@ -128,10 +133,11 @@ public sealed class SyncAllCoordinator(IActivityLog log) await workspace.LoadReposCommand.ExecuteAsync(null); if(!workspace.HasLoadedRepos) { - skipped++; - Announce(workspace, SyncAllAccountState.Skipped); + couldNotSync++; + failed++; + Announce(workspace, SyncAllAccountState.Failed); _log.Info( - $"Sync all: skipped '{workspace.DisplayName}' — its repositories could not be loaded."); + $"Sync all: '{workspace.DisplayName}' could not be synced — its repositories could not be loaded."); continue; } } @@ -151,13 +157,34 @@ public sealed class SyncAllCoordinator(IActivityLog log) // The sync never ran its repositories (its target folder could not be // used, its token went away at that moment): that is not a success with // nothing to report. - skipped++; + couldNotSync++; failed++; Announce(workspace, SyncAllAccountState.Failed); _log.Info($"Sync all: '{workspace.DisplayName}' could not be synced — {workspace.ResultMessage}"); continue; } + if(workspace.ResultKind == RunResultKind.Canceled) + { + /////////////////////////////////////////////////////////////////////////// + // Its own sync was canceled (in its workspace, or because the account // + // was edited meanwhile) and left repositories unsynced: that is not a // + // synced account. The pass itself goes on. // + /////////////////////////////////////////////////////////////////////////// + if(workspace.HasFailedRepos) + { + couldNotSync++; + failed++; + } + else + { + skipped++; + } + Announce(workspace, workspace.HasFailedRepos ? SyncAllAccountState.Failed : SyncAllAccountState.Skipped); + _log.Info($"Sync all: '{workspace.DisplayName}' was canceled before it finished."); + continue; + } + ran++; if(workspace.HasFailedRepos) { @@ -169,7 +196,7 @@ public sealed class SyncAllCoordinator(IActivityLog log) _log.Info($"Sync all: finished '{workspace.DisplayName}'."); } - return new SyncAllResult(ran, skipped, WasCanceled: false, failed); + return new SyncAllResult(ran, skipped, WasCanceled: false, failed, CouldNotSync: couldNotSync); } diff --git a/gclo.ViewModels/SyncAllResult.cs b/gclo.ViewModels/SyncAllResult.cs index 678bfff..cbcbdd5 100644 --- a/gclo.ViewModels/SyncAllResult.cs +++ b/gclo.ViewModels/SyncAllResult.cs @@ -9,11 +9,28 @@ namespace gclo.ViewModels; /// /// Outcome of one sync-all pass over the account workspaces. /// -/// Accounts whose sync ran to completion (successfully or not). -/// Accounts skipped because they were busy, could not load, or had nothing to sync. +/// Accounts whose sync ran to its end (with or without failed repositories). +/// +/// Accounts whose turn came and that were left out with nothing wrong: they were +/// busy, had nothing to sync, or their own sync was canceled. +/// /// True when cancellation stopped the queue before every account was processed. /// -/// Accounts that ended the pass marked as failed: their sync ran and a repository -/// failed, or it could not run at all. The second kind is also in . +/// Accounts that need attention: their sync ran and a repository failed, or they +/// could not be synced. The second kind is counted in too. /// -public sealed record SyncAllResult(int Ran, int Skipped, bool WasCanceled, int Failed = 0); +/// Accounts whose turn never came, because the pass was canceled before it. +/// +/// Accounts whose turn came and that could not be synced: no token, repositories +/// that could not be loaded, a sync that could not start, or their own sync canceled +/// after a repository had failed. +/// +/// +/// , , +/// and count every account of the pass exactly once. +/// +public sealed record SyncAllResult(int Ran, int Skipped, bool WasCanceled, int Failed = 0, int NotStarted = 0, int CouldNotSync = 0) +{ + /// The accounts of the pass. + public int Total => Ran + Skipped + CouldNotSync + NotStarted; +} diff --git a/gclo.ViewModels/SyncAllStatus.cs b/gclo.ViewModels/SyncAllStatus.cs index 0245b83..a91e272 100644 --- a/gclo.ViewModels/SyncAllStatus.cs +++ b/gclo.ViewModels/SyncAllStatus.cs @@ -158,12 +158,29 @@ public sealed partial class SyncAllStatus : ObservableObject /// The sentence that sums up a finished or canceled pass. internal static string Describe(SyncAllResult result) { - string summary = $"{Count(result.Ran, "account")} synced, {result.Skipped.ToString(CultureInfo.InvariantCulture)} skipped."; - return result.Failed switch + /////////////////////////////////////////////////////////////////////////////// + // How many of how many, then each group that was not synced once: an // + // account that could not be synced is one that needs attention, not also // + // one that was "skipped" (#182). // + /////////////////////////////////////////////////////////////////////////////// + string summary = $"{result.Ran.ToString(CultureInfo.InvariantCulture)} of {Count(result.Total, "account")} synced."; + summary += result.Failed switch { - 0 => summary, - 1 => summary + " 1 account needs attention; it is marked in the pane.", - _ => summary + $" {result.Failed.ToString(CultureInfo.InvariantCulture)} accounts need attention; they are marked in the pane.", + 0 => "", + 1 => " 1 account needs attention; it is marked in the pane.", + _ => $" {result.Failed.ToString(CultureInfo.InvariantCulture)} accounts need attention; they are marked in the pane.", + }; + summary += result.Skipped switch + { + 0 => "", + 1 => " 1 account was skipped (busy, canceled, or nothing selected).", + _ => $" {result.Skipped.ToString(CultureInfo.InvariantCulture)} accounts were skipped (busy, canceled, or nothing selected).", + }; + return summary + result.NotStarted switch + { + 0 => "", + 1 => " 1 account was not started.", + _ => $" {result.NotStarted.ToString(CultureInfo.InvariantCulture)} accounts were not started.", }; } diff --git a/gclo.ViewModels/WorkspaceViewModel.cs b/gclo.ViewModels/WorkspaceViewModel.cs index 2481061..f70d04d 100644 --- a/gclo.ViewModels/WorkspaceViewModel.cs +++ b/gclo.ViewModels/WorkspaceViewModel.cs @@ -220,6 +220,8 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable [ObservableProperty] [NotifyCanExecuteChangedFor(nameof(LoadReposCommand))] + [NotifyPropertyChangedFor(nameof(MissingTokenDescription))] + [NotifyPropertyChangedFor(nameof(MissingTokenAdvice))] public partial string Token { get; set; } [ObservableProperty] @@ -604,6 +606,20 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable + /// + /// What an account's page says when the account has no token to work with: where + /// to save one, in the words of the window's own menus. Null when it has one, and + /// for Quick Sync, whose token is typed into its box. + /// + public string? MissingTokenAdvice + => _account is null || Token.Trim().Length > 0 + ? null + : AccountAsItIsNow().UsesDefaultToken + ? "No default token is saved. Save one in File > Settings, or give this account its own token with Edit account…" + : "No token is saved for this account. Choose Edit account… and enter one."; + + + /// /// Tells the workspace that the default token in Settings was saved, replaced, or /// removed. An account workspace takes its token from its source again. Quick @@ -967,7 +983,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable RebuildReposIndex(); LoadedOrganization = organization; HasLoadedRepos = true; - StatusText = $"{Repos.Count} repositories loaded."; + StatusText = Repos.Count == 1 ? "1 repository loaded." : $"{Repos.Count} repositories loaded."; _log.Info($"Loaded {Repos.Count} repositories for organization '{organization}'."); } catch(Exception ex) @@ -1016,7 +1032,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable // token leaves the table (failures, their paths, the retry) as it was. string token = TokenToUseNow(); - var selected = Repos.Where(r => r.IsSelected).ToList(); + List selected = _runOnly ?? [.. Repos.Where(r => r.IsSelected)]; foreach(RepoItemViewModel item in selected) { item.Status = SyncStatus.Queued; @@ -1032,7 +1048,7 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable RebuildFilteredRepos(); // every selected row just went back to Queued string targetRoot = EffectiveTargetRoot; - StatusText = $"Syncing {selected.Count} repositories..."; + StatusText = selected.Count == 1 ? "Syncing 1 repository..." : $"Syncing {selected.Count} repositories..."; _log.Info($"Sync started: {selected.Count} repositories into '{targetRoot}'."); // Constructed on the UI thread: the delivery captures the WinUI @@ -1129,34 +1145,38 @@ public sealed partial class WorkspaceViewModel : ObservableObject, IDisposable - /// Selects exactly the failed repositories and runs the same sync path again. + /// + /// Runs the same sync path again for exactly the failed repositories. The + /// selection is not touched: the retry is one run, not a change of what the next + /// sync covers. (It used to select the failed rows; a later Sync, or Sync all, + /// then synced only those and reported the account as synced, #182.) + /// [RelayCommand(CanExecute = nameof(CanRetryFailed))] private async Task RetryFailedAsync() { - _log.Info($"Retrying {Repos.Count(r => r.Status == SyncStatus.Failed)} failed repositories."); - // One batch: per-row recounts and command re-queries would be O(n^2) (#30). - _syncingSelection = true; + _runOnly = [.. Repos.Where(r => r.Status == SyncStatus.Failed)]; + _log.Info($"Retrying {_runOnly.Count} failed repositories."); try { - foreach(RepoItemViewModel item in Repos) - { - item.IsSelected = item.Status == SyncStatus.Failed; - } + // Executing SyncCommand itself keeps SyncCancelCommand working for retry runs. + await SyncCommand.ExecuteAsync(null); } finally { - _syncingSelection = false; + _runOnly = null; } - RecomputeSelectedCount(); - UpdateAllSelectedFromItems(); - SyncCommand.NotifyCanExecuteChanged(); - - // Executing SyncCommand itself keeps SyncCancelCommand working for retry runs. - await SyncCommand.ExecuteAsync(null); } + /// + /// The repositories the run that is starting takes instead of the selected ones; + /// null for an ordinary sync. Set by for its one run. + /// + private List? _runOnly; + + + // ---------------------------------------------------------------- path recovery /// diff --git a/gclo/ConnectionFormControl.xaml.cs b/gclo/ConnectionFormControl.xaml.cs index 2f0ca93..b301916 100644 --- a/gclo/ConnectionFormControl.xaml.cs +++ b/gclo/ConnectionFormControl.xaml.cs @@ -86,15 +86,30 @@ public sealed partial class ConnectionFormControl : UserControl private void OnLoaded(object sender, RoutedEventArgs e) { bool hasToken = ViewModel is not null && ViewModel.Token.Length > 0; + bool savedWithAnAccount = ViewModel?.AccountId is not null; TokenBox.Password = hasToken ? SettingsTokenState.Mask : ""; - TokenBox.PlaceholderText = "ghp_…"; - // The organization placeholder must not ask for a token the box already has. - OrgBox.PlaceholderText = hasToken ? "Choose an organization, or type one" : "Paste a token first, or type an org"; - ToolTipService.SetToolTip(TokenBox, !hasToken - ? "Fine-grained or classic PAT with repository read access; used for the GitHub API and git over HTTPS. Never written to disk or logs." - : ViewModel!.AccountId is null - ? "Using the saved default token from Settings. Type here to use a different one for this session." - : "Using this account's token. Type here to use a different one for this session."); + + /////////////////////////////////////////////////////////////////////////////// + // An account's token is read from where it is saved each time it is used // + // (#158). A token typed here would look taken and then not be used, so the // + // box of an account takes none; the page says where the token is changed // + // (#182). // + /////////////////////////////////////////////////////////////////////////////// + TokenBox.IsEnabled = !savedWithAnAccount; + TokenBox.PlaceholderText = !savedWithAnAccount ? "ghp_…" + : ViewModel!.UsesDefaultToken ? "No default token is saved" + : "No token is saved for this account"; + // The organization placeholder must not ask for a token the box already has, or cannot take. + OrgBox.PlaceholderText = hasToken ? "Choose an organization, or type one" + : savedWithAnAccount ? "Type an organization or account" + : "Paste a token first, or type an org"; + ToolTipService.SetToolTip(TokenBox, savedWithAnAccount + ? ViewModel!.UsesDefaultToken + ? "This account uses the default token from File > Settings." + : "This account's token is set with Edit account…, not typed here." + : !hasToken + ? "Fine-grained or classic PAT with repository read access; used for the GitHub API and git over HTTPS. Never written to disk or logs." + : "Using the saved default token from Settings. Type here to use a different one for this session."); } @@ -105,7 +120,7 @@ public sealed partial class ConnectionFormControl : UserControl private void TokenBox_PasswordChanged(object sender, RoutedEventArgs e) { string typed = ((PasswordBox)sender).Password; - if(ViewModel is not null && !string.Equals(typed, SettingsTokenState.Mask, StringComparison.Ordinal)) + if(ViewModel is not null && ViewModel.AccountId is null && !string.Equals(typed, SettingsTokenState.Mask, StringComparison.Ordinal)) { ViewModel.Token = typed; } diff --git a/gclo/MainWindow.xaml.cs b/gclo/MainWindow.xaml.cs index 8030d31..5cfddba 100644 --- a/gclo/MainWindow.xaml.cs +++ b/gclo/MainWindow.xaml.cs @@ -608,11 +608,13 @@ public sealed partial class MainWindow : Window CreateBadge("InformationalDotInfoBadgeStyle", "SystemFillColorSolidNeutralBrush"), SyncAllAccountState.Succeeded => CreateBadge("SuccessIconInfoBadgeStyle", "SystemFillColorSuccessBrush"), - SyncAllAccountState.Failed => + SyncAllAccountState.Failed when viewModel.HasFailedRepos => CreateBadge( "CriticalValueInfoBadgeStyle", "SystemFillColorCriticalBrush", viewModel.Repos.Count(r => r.Status == SyncStatus.Failed)), + SyncAllAccountState.Failed => // it could not be synced at all: there is no number to show + CreateBadge("CriticalDotInfoBadgeStyle", "SystemFillColorCriticalBrush"), SyncAllAccountState.Skipped => CreateBadge("CautionDotInfoBadgeStyle", "SystemFillColorCautionBrush"), _ => null, // Running is covered by IsRunning above diff --git a/gclo/WorkspacePage.xaml b/gclo/WorkspacePage.xaml index 2e86760..75fd444 100644 --- a/gclo/WorkspacePage.xaml +++ b/gclo/WorkspacePage.xaml @@ -129,6 +129,22 @@ TextWrapping="Wrap" Text="Once this connection works, save it as an account (Edit on the connection chip) so the token is kept for you." Visibility="{x:Bind local:StatusFormat.VisibleIfNot(IsAccountWorkspace), Mode=OneTime}" /> + + + + + public string AccountTokenNote + => ViewModel.UsesDefaultToken + ? "This account uses the default token from File > Settings." + : "This account's token is set with Edit account…, not typed here."; + + + + /// Whether the connect card says that the account has no token, and where to save one. + public Visibility MissingTokenVisibility(string? advice) + => string.IsNullOrEmpty(advice) ? Visibility.Collapsed : Visibility.Visible; + + + /// /// Unhooks every subscription this page made on its (longer-lived) view model, so /// a page the shell releases while its workspace is idle can be collected: the @@ -216,6 +230,11 @@ public sealed partial class WorkspacePage : UserControl // it here. Enqueued so the binding has pushed the new text first. DispatcherQueue.TryEnqueue(RaiseStatusLiveRegionChanged); } + else if(string.Equals(e.PropertyName, nameof(WorkspaceViewModel.MissingTokenAdvice), StringComparison.Ordinal)) + { + // The same goes for the connect card's line about a missing token. + DispatcherQueue.TryEnqueue(() => RaiseLiveRegionChanged(MissingTokenTextBlock)); + } else if(e.PropertyName is nameof(WorkspaceViewModel.SelectedCount) or nameof(WorkspaceViewModel.LoadedOrganization)) { @@ -445,10 +464,14 @@ public sealed partial class WorkspacePage : UserControl - private void RaiseStatusLiveRegionChanged() + private void RaiseStatusLiveRegionChanged() => RaiseLiveRegionChanged(StatusTextBlock); + + + + private static void RaiseLiveRegionChanged(FrameworkElement element) { - AutomationPeer peer = Microsoft.UI.Xaml.Automation.Peers.FrameworkElementAutomationPeer.FromElement(StatusTextBlock) - ?? Microsoft.UI.Xaml.Automation.Peers.FrameworkElementAutomationPeer.CreatePeerForElement(StatusTextBlock); + AutomationPeer peer = Microsoft.UI.Xaml.Automation.Peers.FrameworkElementAutomationPeer.FromElement(element) + ?? Microsoft.UI.Xaml.Automation.Peers.FrameworkElementAutomationPeer.CreatePeerForElement(element); peer?.RaiseAutomationEvent(Microsoft.UI.Xaml.Automation.Peers.AutomationEvents.LiveRegionChanged); }