diff --git a/src/DodoSSH.Client.App/Views/StatusBar.axaml b/src/DodoSSH.Client.App/Views/StatusBar.axaml index 212a897..c8768a7 100644 --- a/src/DodoSSH.Client.App/Views/StatusBar.axaml +++ b/src/DodoSSH.Client.App/Views/StatusBar.axaml @@ -25,6 +25,14 @@ channel this application has for saying that a save failed, that a sync was refused, that a merge picked a winner. The design is a mock-up of a working afternoon and has nowhere for a sentence like that to go; dropping the bar would have meant dropping the sentence, or repeating it on six screens. + + ── One line, several terminals ──────────────────────────────────────────────────────────────────────── + One bar for a window that now holds any number of tabs, and connecting writes to it — so "Connected to + prod-db." outlived the tab it was about, and closing prod-db left that sentence sitting under whichever + terminal the user looked at next. A connection's own lines are now owned by the attempt that wrote them + and are cleared when its tab is closed; everything else the vault says stays, because closing a terminal + is not an answer to a failed save. See VaultViewModel.ForgetConnectionStatus and + MainWindowViewModel.CloseTabAsync. --> /// + /// /// This is the one thing in the application that deliberately ends a session, which is why it is a tab's /// close button and not a menu item: closing the window somebody's job is running in should take exactly /// as much intent as it looks like it does. Locking does not do this, and neither does anything else. + /// + /// + /// It takes the status line with it, when that line is this tab's. The strip along the bottom is + /// one line for the whole window — see StatusBar.axaml — and connecting writes to it, so closing + /// prod-db used to leave "Connected to prod-db." reporting on a session that is gone, over whichever + /// terminal the user looked at next. Only the sentence this attempt wrote goes; anything the vault has + /// said since stays, because a failed save is not something closing a terminal answers. + /// /// [RelayCommand] private async Task CloseTabAsync(TerminalTabViewModel tab) @@ -1960,6 +1969,9 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp RaiseTabState(); + // Only this tab's own sentence, and only if that is what the bar currently holds — see the remark. + Vault?.ForgetConnectionStatus(tab.AttemptId); + // Explicitly, and not left to the selection having moved. Closing a tab that was not the selected one // changes nothing about the selection, so OnSelectedTabChanged does not run — and the host whose // terminal just went would keep a lit dot until something else happened to move the selection. @@ -3400,7 +3412,7 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp /// private void OnVaultConnectionStarting(object? sender, ConnectionAttemptEventArgs e) { - var tab = new TerminalTabViewModel(e.Label, e.Address); + var tab = new TerminalTabViewModel(e.Label, e.Address) { AttemptId = e.AttemptId }; attempts[e.AttemptId] = tab; AdoptTab(tab); @@ -3429,6 +3441,7 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp // dropping it would leave a shell running with nothing in the window naming it. var adopted = new TerminalTabViewModel(e.SessionId, e.Label, e.Address) { + AttemptId = e.AttemptId, StartedAt = clock.GetUtcNow(), Cipher = NullIfEmpty(e.Cipher), HostKeyAlgorithm = NullIfEmpty(e.HostKeyAlgorithm), diff --git a/src/DodoSSH.Client.Shell/ViewModels/TerminalTabViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/TerminalTabViewModel.cs index 4b6d07a..b146d43 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/TerminalTabViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/TerminalTabViewModel.cs @@ -84,6 +84,18 @@ internal sealed partial class TerminalTabViewModel : ObservableObject /// internal uint SessionId { get; private set; } + /// + /// The connection attempt this tab was opened for, or for a tab nothing dialled. + /// + /// + /// Carried only so that closing this tab can take the vault's status line with it when the line is about + /// this connection — see VaultViewModel.ForgetConnectionStatus. It is the attempt id and not the + /// session id because the message can be written before a session exists ("Connecting to prod-db…") and + /// after one has stopped existing, and because the two failure sentences a tab can end on never have a + /// session id at all. + /// + internal Guid AttemptId { get; init; } + internal string Label { get; } /// The account and endpoint, for the pane header and the status bar. diff --git a/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs index 48fc4d7..8f75382 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/VaultViewModel.cs @@ -2130,6 +2130,60 @@ internal sealed partial class VaultViewModel( [ObservableProperty] private string status = string.Empty; + /// + /// The connection is currently about, or when it is about + /// something else — a save, a sync, a refusal that has nothing to do with a terminal. + /// + /// + /// + /// The status line is one line for the whole window, and most of what goes through it is about the vault, + /// which is one thing however many terminals are open. A connection is not: "Connected to prod-db." is + /// about one tab, and it used to outlive that tab — close prod-db, look at the terminal beside it, and the + /// strip along the bottom was still reporting on a session that is no longer there. + /// + /// + /// Ownership rather than a blanket clear on every close, because the alternative loses the sentences this + /// bar exists for: a failed save or a refused sync is what the design deleted the bar's other four fields + /// to make room for, and closing a terminal is no reason to take one away. + /// + /// + /// Reset by on every write, so the only messages that carry an owner are the + /// ones writes. Anything assigning in the ordinary way says, + /// by doing so, that its message belongs to no tab. + /// + /// + private Guid statusAttemptId; + + partial void OnStatusChanged(string value) => statusAttemptId = Guid.Empty; + + /// Writes a status line that belongs to one connection attempt. + /// + /// After the assignment, not before: clears the owner on every write, which + /// is what makes "belongs to nobody" the default rather than something each of the other status writes + /// would have to remember to say. + /// + private void SayAbout(ConnectionAttemptEventArgs attempt, string message) + { + Status = message; + statusAttemptId = attempt.AttemptId; + } + + /// + /// Clears the status line if what it is saying is about this connection, and leaves it alone otherwise. + /// + /// + /// Called by the shell when a tab is closed — see MainWindowViewModel.CloseTabAsync. Cleared to + /// empty rather than replaced with "Closed prod-db.": the tab going is the report, it is the report the + /// user just made happen, and a bar that answers every close with a sentence is a bar people stop reading. + /// + internal void ForgetConnectionStatus(Guid attemptId) + { + if (attemptId != Guid.Empty && statusAttemptId == attemptId) + { + Status = string.Empty; + } + } + [ObservableProperty] private int pendingChanges; @@ -10767,7 +10821,7 @@ internal sealed partial class VaultViewModel( Dialled(target, authentication)); ConnectionStarting?.Invoke(this, attempt); - Status = $"Connecting to {target.Label}…"; + SayAbout(attempt, $"Connecting to {target.Label}…"); await OpenSessionAsync(attempt, target, authentication, cancellationToken).ConfigureAwait(true); } @@ -11042,7 +11096,9 @@ internal sealed partial class VaultViewModel( /// private void Abandon(ConnectionAttemptEventArgs attempt, string reason) { - Status = reason; + // Owned by the attempt, unlike Answer's own line below: this is the case where the tab stays, carrying + // the same reason, so closing that tab is exactly the moment the line stops being about anything. + SayAbout(attempt, reason); ConnectionFailed?.Invoke( this, @@ -11097,7 +11153,7 @@ internal sealed partial class VaultViewModel( // only record that machine was reached at all. connectionLog?.Identify(sessionId, target.Label, target.HostId); - Status = $"Connected to {target.Label}."; + SayAbout(attempt, $"Connected to {target.Label}."); // The session the workspace just opened is the only place the negotiated cipher and host-key // algorithm live — an SshConnectionRequest asks for neither and gets no say in either — so they are @@ -11128,7 +11184,8 @@ internal sealed partial class VaultViewModel( // screen it is typed on says so. if (target.Row is { } row) { - await RememberTypedPasswordAsync(row, authentication, cancellationToken).ConfigureAwait(true); + await RememberTypedPasswordAsync(attempt, row, authentication, cancellationToken) + .ConfigureAwait(true); } } @@ -11160,6 +11217,7 @@ internal sealed partial class VaultViewModel( /// /// private async Task RememberTypedPasswordAsync( + ConnectionAttemptEventArgs attempt, HostRowViewModel row, HostAuthentication authentication, CancellationToken cancellationToken) @@ -11175,8 +11233,10 @@ internal sealed partial class VaultViewModel( if (row.IsReadOnly) { - Status = $"Connected to {row.Label}. Its password was not saved: this host was written by a " - + "newer version of DodoSSH, and binding a credential would re-encode it."; + SayAbout( + attempt, + $"Connected to {row.Label}. Its password was not saved: this host was written by a newer " + + "version of DodoSSH, and binding a credential would re-encode it."); return; } @@ -11200,7 +11260,9 @@ internal sealed partial class VaultViewModel( } catch (Exception exception) { - Status = $"Connected to {row.Label}, but its password could not be saved: {exception.Message}"; + SayAbout( + attempt, + $"Connected to {row.Label}, but its password could not be saved: {exception.Message}"); return; } @@ -11211,8 +11273,10 @@ internal sealed partial class VaultViewModel( await ReloadAsync(cancellationToken).ConfigureAwait(true); - Status = $"Connected to {row.Label}. Its password is saved in your keychain as '{row.Label}', so it " - + "will not be asked for again."; + SayAbout( + attempt, + $"Connected to {row.Label}. Its password is saved in your keychain as '{row.Label}', so it will " + + "not be asked for again."); await AutoSyncAsync(cancellationToken).ConfigureAwait(true); } diff --git a/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs b/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs index 9cb60be..57611cf 100644 --- a/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs +++ b/tests/DodoSSH.Client.App.Tests/ShellFlowTests.cs @@ -1254,6 +1254,68 @@ public sealed class ShellFlowTests : IAsyncLifetime shell.IsTerminalShowing.ShouldBeTrue(); } + /// + /// The status line is one line for the whole window, so a sentence about a session that has been closed is + /// a sentence the user reads over some other terminal. + /// + [Fact] + public async Task ClosingATab_TakesItsOwnStatusLineWithIt() + { + var vault = await ReadyToConnectAsync(); + + await using var renderer = await FakeRenderer.AttachAsync(workspace, Token); + + await vault.ConnectCommand.ExecuteAsync(null); + vault.Status.ShouldContain("Connected", Case.Insensitive); + + await shell.CloseTabCommand.ExecuteAsync(shell.Tabs[0]); + + vault.Status.ShouldBeEmpty(); + } + + /// + /// The other half, and the reason closing does not simply blank the bar: everything this application has + /// to say about a save, a sync or a refusal goes through the same line — see StatusBar.axaml — and + /// closing a terminal answers none of it. + /// + [Fact] + public async Task ClosingATab_LeavesAStatusLineThatIsAboutSomethingElse() + { + var vault = await ReadyToConnectAsync(); + + await using var renderer = await FakeRenderer.AttachAsync(workspace, Token); + + await vault.ConnectCommand.ExecuteAsync(null); + vault.Status = "The keychain could not be saved."; + + await shell.CloseTabCommand.ExecuteAsync(shell.Tabs[0]); + + vault.Status.ShouldBe("The keychain could not be saved."); + } + + /// + /// Two sessions to the same host, which is the case a label could not tell apart: the line belongs to the + /// attempt that wrote it, so closing the other one leaves it alone. + /// + [Fact] + public async Task ClosingATab_LeavesTheLineAnotherTabWrote() + { + var vault = await ReadyToConnectAsync(); + + await using var renderer = await FakeRenderer.AttachAsync(workspace, Token); + + await vault.ConnectCommand.ExecuteAsync(null); + var first = shell.Tabs[0]; + + // The second connection's own line is what the bar holds now. + await vault.ConnectCommand.ExecuteAsync(null); + var reported = vault.Status; + + await shell.CloseTabCommand.ExecuteAsync(first); + + vault.Status.ShouldBe(reported); + } + // ---- Connecting, while it is still happening ---- // // A handshake is a network round trip and no longer holds the vault while it runs, so there is a stretch