Public Access
Let a closed terminal take its own status line with it
The strip along the bottom is one line for the whole window, and connecting writes to it, so "Connected to prod-db." outlived the tab it was about: close prod-db, look at the terminal beside it, and the bar was still reporting on a session that is no longer there. A connection's own lines are now owned by the attempt that wrote them, and closing a tab clears the line only when that is what the bar is holding. Ownership rather than a blanket clear on 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 answer to one.
This commit is contained in:
@@ -25,6 +25,14 @@
|
|||||||
channel this application has for saying that a save failed, that a sync was refused, that a merge picked
|
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;
|
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.
|
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.
|
||||||
-->
|
-->
|
||||||
|
|
||||||
<Border Height="24" Background="{StaticResource Chrome}"
|
<Border Height="24" Background="{StaticResource Chrome}"
|
||||||
|
|||||||
@@ -1924,9 +1924,18 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp
|
|||||||
/// Closes one terminal, ending its shell.
|
/// Closes one terminal, ending its shell.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
/// <remarks>
|
/// <remarks>
|
||||||
|
/// <para>
|
||||||
/// This is the one thing in the application that deliberately ends a session, which is why it is a tab's
|
/// 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
|
/// 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.
|
/// as much intent as it looks like it does. Locking does not do this, and neither does anything else.
|
||||||
|
/// </para>
|
||||||
|
/// <para>
|
||||||
|
/// <b>It takes the status line with it, when that line is this tab's.</b> The strip along the bottom is
|
||||||
|
/// one line for the whole window — see <c>StatusBar.axaml</c> — 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.
|
||||||
|
/// </para>
|
||||||
/// </remarks>
|
/// </remarks>
|
||||||
[RelayCommand]
|
[RelayCommand]
|
||||||
private async Task CloseTabAsync(TerminalTabViewModel tab)
|
private async Task CloseTabAsync(TerminalTabViewModel tab)
|
||||||
@@ -1960,6 +1969,9 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp
|
|||||||
|
|
||||||
RaiseTabState();
|
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
|
// 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
|
// 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.
|
// 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
|
|||||||
/// </remarks>
|
/// </remarks>
|
||||||
private void OnVaultConnectionStarting(object? sender, ConnectionAttemptEventArgs e)
|
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;
|
attempts[e.AttemptId] = tab;
|
||||||
|
|
||||||
AdoptTab(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.
|
// dropping it would leave a shell running with nothing in the window naming it.
|
||||||
var adopted = new TerminalTabViewModel(e.SessionId, e.Label, e.Address)
|
var adopted = new TerminalTabViewModel(e.SessionId, e.Label, e.Address)
|
||||||
{
|
{
|
||||||
|
AttemptId = e.AttemptId,
|
||||||
StartedAt = clock.GetUtcNow(),
|
StartedAt = clock.GetUtcNow(),
|
||||||
Cipher = NullIfEmpty(e.Cipher),
|
Cipher = NullIfEmpty(e.Cipher),
|
||||||
HostKeyAlgorithm = NullIfEmpty(e.HostKeyAlgorithm),
|
HostKeyAlgorithm = NullIfEmpty(e.HostKeyAlgorithm),
|
||||||
|
|||||||
@@ -84,6 +84,18 @@ internal sealed partial class TerminalTabViewModel : ObservableObject
|
|||||||
/// </remarks>
|
/// </remarks>
|
||||||
internal uint SessionId { get; private set; }
|
internal uint SessionId { get; private set; }
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// The connection attempt this tab was opened for, or <see cref="Guid.Empty"/> for a tab nothing dialled.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// Carried only so that closing this tab can take the vault's status line with it when the line is about
|
||||||
|
/// this connection — see <c>VaultViewModel.ForgetConnectionStatus</c>. 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.
|
||||||
|
/// </remarks>
|
||||||
|
internal Guid AttemptId { get; init; }
|
||||||
|
|
||||||
internal string Label { get; }
|
internal string Label { get; }
|
||||||
|
|
||||||
/// <summary>The account and endpoint, for the pane header and the status bar.</summary>
|
/// <summary>The account and endpoint, for the pane header and the status bar.</summary>
|
||||||
|
|||||||
@@ -2130,6 +2130,60 @@ internal sealed partial class VaultViewModel(
|
|||||||
[ObservableProperty]
|
[ObservableProperty]
|
||||||
private string status = string.Empty;
|
private string status = string.Empty;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// The connection <see cref="Status"/> is currently about, or <see cref="Guid.Empty"/> when it is about
|
||||||
|
/// something else — a save, a sync, a refusal that has nothing to do with a terminal.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// <para>
|
||||||
|
/// 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.
|
||||||
|
/// </para>
|
||||||
|
/// <para>
|
||||||
|
/// 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.
|
||||||
|
/// </para>
|
||||||
|
/// <para>
|
||||||
|
/// Reset by <see cref="OnStatusChanged"/> on every write, so the only messages that carry an owner are the
|
||||||
|
/// ones <see cref="SayAbout"/> writes. Anything assigning <see cref="Status"/> in the ordinary way says,
|
||||||
|
/// by doing so, that its message belongs to no tab.
|
||||||
|
/// </para>
|
||||||
|
/// </remarks>
|
||||||
|
private Guid statusAttemptId;
|
||||||
|
|
||||||
|
partial void OnStatusChanged(string value) => statusAttemptId = Guid.Empty;
|
||||||
|
|
||||||
|
/// <summary>Writes a status line that belongs to one connection attempt.</summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// After the assignment, not before: <see cref="OnStatusChanged"/> 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.
|
||||||
|
/// </remarks>
|
||||||
|
private void SayAbout(ConnectionAttemptEventArgs attempt, string message)
|
||||||
|
{
|
||||||
|
Status = message;
|
||||||
|
statusAttemptId = attempt.AttemptId;
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Clears the status line if what it is saying is about this connection, and leaves it alone otherwise.
|
||||||
|
/// </summary>
|
||||||
|
/// <remarks>
|
||||||
|
/// Called by the shell when a tab is closed — see <c>MainWindowViewModel.CloseTabAsync</c>. 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.
|
||||||
|
/// </remarks>
|
||||||
|
internal void ForgetConnectionStatus(Guid attemptId)
|
||||||
|
{
|
||||||
|
if (attemptId != Guid.Empty && statusAttemptId == attemptId)
|
||||||
|
{
|
||||||
|
Status = string.Empty;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
[ObservableProperty]
|
[ObservableProperty]
|
||||||
private int pendingChanges;
|
private int pendingChanges;
|
||||||
|
|
||||||
@@ -10767,7 +10821,7 @@ internal sealed partial class VaultViewModel(
|
|||||||
Dialled(target, authentication));
|
Dialled(target, authentication));
|
||||||
|
|
||||||
ConnectionStarting?.Invoke(this, attempt);
|
ConnectionStarting?.Invoke(this, attempt);
|
||||||
Status = $"Connecting to {target.Label}…";
|
SayAbout(attempt, $"Connecting to {target.Label}…");
|
||||||
|
|
||||||
await OpenSessionAsync(attempt, target, authentication, cancellationToken).ConfigureAwait(true);
|
await OpenSessionAsync(attempt, target, authentication, cancellationToken).ConfigureAwait(true);
|
||||||
}
|
}
|
||||||
@@ -11042,7 +11096,9 @@ internal sealed partial class VaultViewModel(
|
|||||||
/// </remarks>
|
/// </remarks>
|
||||||
private void Abandon(ConnectionAttemptEventArgs attempt, string reason)
|
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(
|
ConnectionFailed?.Invoke(
|
||||||
this,
|
this,
|
||||||
@@ -11097,7 +11153,7 @@ internal sealed partial class VaultViewModel(
|
|||||||
// only record that machine was reached at all.
|
// only record that machine was reached at all.
|
||||||
connectionLog?.Identify(sessionId, target.Label, target.HostId);
|
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
|
// 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
|
// 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.
|
// screen it is typed on says so.
|
||||||
if (target.Row is { } row)
|
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(
|
|||||||
/// </para>
|
/// </para>
|
||||||
/// </remarks>
|
/// </remarks>
|
||||||
private async Task RememberTypedPasswordAsync(
|
private async Task RememberTypedPasswordAsync(
|
||||||
|
ConnectionAttemptEventArgs attempt,
|
||||||
HostRowViewModel row,
|
HostRowViewModel row,
|
||||||
HostAuthentication authentication,
|
HostAuthentication authentication,
|
||||||
CancellationToken cancellationToken)
|
CancellationToken cancellationToken)
|
||||||
@@ -11175,8 +11233,10 @@ internal sealed partial class VaultViewModel(
|
|||||||
|
|
||||||
if (row.IsReadOnly)
|
if (row.IsReadOnly)
|
||||||
{
|
{
|
||||||
Status = $"Connected to {row.Label}. Its password was not saved: this host was written by a "
|
SayAbout(
|
||||||
+ "newer version of DodoSSH, and binding a credential would re-encode it.";
|
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;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -11200,7 +11260,9 @@ internal sealed partial class VaultViewModel(
|
|||||||
}
|
}
|
||||||
catch (Exception exception)
|
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;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -11211,8 +11273,10 @@ internal sealed partial class VaultViewModel(
|
|||||||
|
|
||||||
await ReloadAsync(cancellationToken).ConfigureAwait(true);
|
await ReloadAsync(cancellationToken).ConfigureAwait(true);
|
||||||
|
|
||||||
Status = $"Connected to {row.Label}. Its password is saved in your keychain as '{row.Label}', so it "
|
SayAbout(
|
||||||
+ "will not be asked for again.";
|
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);
|
await AutoSyncAsync(cancellationToken).ConfigureAwait(true);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1254,6 +1254,68 @@ public sealed class ShellFlowTests : IAsyncLifetime
|
|||||||
shell.IsTerminalShowing.ShouldBeTrue();
|
shell.IsTerminalShowing.ShouldBeTrue();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <remarks>
|
||||||
|
/// 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.
|
||||||
|
/// </remarks>
|
||||||
|
[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();
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <remarks>
|
||||||
|
/// 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 <c>StatusBar.axaml</c> — and
|
||||||
|
/// closing a terminal answers none of it.
|
||||||
|
/// </remarks>
|
||||||
|
[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.");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <remarks>
|
||||||
|
/// 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.
|
||||||
|
/// </remarks>
|
||||||
|
[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 ----
|
// ---- 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
|
// A handshake is a network round trip and no longer holds the vault while it runs, so there is a stretch
|
||||||
|
|||||||
Reference in New Issue
Block a user