mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-22 00:02:23 +00:00
fix(sftp,forwards,files): tell a failure apart from an empty result (#518)
* fix(sftp): stop a failed poll from reading as an empty transfer list
A transfer poll that could not reach the daemon answered with an empty
`Vec`, which is indistinguishable from "every transfer is gone": the tray
disappeared and every upload the panel was waiting on counted as landed,
so a spurious "the upload finished" refresh fired. Over a link that is
down that is the permanent answer, not a blink.
`SftpRoute::transfer_list` and `RemoteTerminal::sftp_transfer_list` now
report the failure the way `sftp_list` already does. A failed poll keeps
the jobs the panel last saw, settles nothing, and says so in the transfer
tray through a new `jobs_error` — kept apart from `SftpPanelState::error`,
which blanks the directory listing a poll knows nothing about.
* fix(forwards): let a forward whose loop has exited say so
`ForwardEntry.status` was written once when the forward was set up and
never touched again, so a local or dynamic forward went on reporting
`Listening` after its accept loop had already exited. The pane outlives a
dead SSH transport on purpose, the daemon keeps it while a subscriber is
attached, and the panel re-polls every 2s — so the stale `Listening` is
not a blink but the permanent answer. `nc` to the port gets accepted once
and refused thereafter while the panel still shows it as live.
The status is now an `Arc<Mutex<ForwardStatus>>` shared with the task, and
both break arms record why they left: the listening socket closed, or the
SSH connection went away. `ForwardStatus::Error` carries it rather than a
new variant, because the enum crosses the protocol to `tty7-server` builds
that would not know one. `find_auto_local` reads the live status too, so a
loopback link is no longer reused after its forward has stopped serving.
A remote forward has no accept loop of its own — the far end opens the
channels — so it keeps whatever the `tcpip-forward` request answered.
* fix(files): tell a failed search from an empty one, and name the file a write failed on
Two ways the file tree answered a failure with something that reads as a
result.
A search was `unwrap_or_default()`ed inside the worker, so a host that
refused the walk left `hits` empty and the column printed "Nothing matches
{query}" — byte-identical to a genuine zero-hit search. The worker now
reports `(ok, hits)` the way `spawn_load` already reports a listing, and a
failed walk draws a `SearchFailed` note in the danger colour, the same
distinction `FileTreeState.unreadable` draws for a directory.
Creating and renaming pushed the bare `io::Error`, so the toast was
literally "Permission denied (os error 13)" — neither which file nor what
was being done to it. Both now go through `HostOps::notify_err` like
delete and drop-copy already do, naming the file; a rename names the name
it is leaving, which is the one still on screen.
---------
Co-authored-by: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com>
This commit is contained in:
@@ -199,6 +199,53 @@ enum ForwardCancel {
|
||||
None,
|
||||
}
|
||||
|
||||
/// A forward's status, shared with the task that serves it.
|
||||
///
|
||||
/// The status used to be a plain field written once when the forward was set
|
||||
/// up and never again, so a forward whose accept loop had already exited went
|
||||
/// on reporting `Listening` for as long as the pane stayed open — which is
|
||||
/// forever, because a pane is deliberately not closed when its SSH connection
|
||||
/// dies. The task that discovers the truth is the one that has to be able to
|
||||
/// record it.
|
||||
type SharedStatus = Arc<Mutex<ForwardStatus>>;
|
||||
|
||||
/// Why a forward's accept loop stopped.
|
||||
enum LoopExit {
|
||||
/// `accept()` failed over and over: the listening socket is no longer
|
||||
/// usable, so nothing can even reach the forward any more.
|
||||
ListenerLost,
|
||||
/// The SSH transport went away under it. The port may still be bound —
|
||||
/// a connection to it is accepted and then dropped — but there is nothing
|
||||
/// on the far side of it.
|
||||
ConnectionLost,
|
||||
}
|
||||
|
||||
/// The status a forward is left in once its loop has exited.
|
||||
///
|
||||
/// `ForwardStatus::Error` rather than a `Stopped` variant of its own, on
|
||||
/// purpose: `ForwardStatus` is serialised across the protocol to remote
|
||||
/// `tty7-server` builds, and a variant an older remote has never heard of is a
|
||||
/// deserialisation failure rather than an unknown status. `Error` already
|
||||
/// draws as a danger badge with its message beside it.
|
||||
fn loop_exit_status(exit: LoopExit) -> ForwardStatus {
|
||||
ForwardStatus::Error(
|
||||
match exit {
|
||||
LoopExit::ListenerLost => "stopped: the listening socket closed",
|
||||
LoopExit::ConnectionLost => "stopped: the SSH connection went away",
|
||||
}
|
||||
.to_string(),
|
||||
)
|
||||
}
|
||||
|
||||
/// A forward that never got as far as a listening socket. It has no task, so
|
||||
/// nothing will ever move it off this status.
|
||||
fn bind_failed(rule: &SshForwardRule, e: io::Error) -> SharedStatus {
|
||||
Arc::new(Mutex::new(ForwardStatus::Error(format!(
|
||||
"bind {}:{} failed: {e}",
|
||||
rule.bind_host, rule.bind_port
|
||||
))))
|
||||
}
|
||||
|
||||
struct ForwardEntry {
|
||||
id: u64,
|
||||
kind: SshForwardKind,
|
||||
@@ -207,7 +254,7 @@ struct ForwardEntry {
|
||||
target_host: String,
|
||||
target_port: u16,
|
||||
description: Option<String>,
|
||||
status: ForwardStatus,
|
||||
status: SharedStatus,
|
||||
cancel: ForwardCancel,
|
||||
auto_local: bool,
|
||||
}
|
||||
@@ -229,7 +276,7 @@ impl ForwardEntry {
|
||||
target_host: self.target_host.clone(),
|
||||
target_port: self.target_port,
|
||||
description: self.description.clone(),
|
||||
status: self.status.clone(),
|
||||
status: self.status.lock().unwrap().clone(),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -390,18 +437,11 @@ impl SshForwardRegistry {
|
||||
&self,
|
||||
conn: &Arc<SshConnection>,
|
||||
rule: &SshForwardRule,
|
||||
) -> (u16, ForwardStatus, ForwardCancel) {
|
||||
) -> (u16, SharedStatus, ForwardCancel) {
|
||||
let listener = match TcpListener::bind((rule.bind_host.as_str(), rule.bind_port)).await {
|
||||
Ok(l) => l,
|
||||
Err(e) => {
|
||||
return (
|
||||
rule.bind_port,
|
||||
ForwardStatus::Error(format!(
|
||||
"bind {}:{} failed: {e}",
|
||||
rule.bind_host, rule.bind_port
|
||||
)),
|
||||
ForwardCancel::None,
|
||||
);
|
||||
return (rule.bind_port, bind_failed(rule, e), ForwardCancel::None);
|
||||
}
|
||||
};
|
||||
let bound = listener
|
||||
@@ -411,14 +451,16 @@ impl SshForwardRegistry {
|
||||
let conn = conn.clone();
|
||||
let target_host = rule.target_host.clone();
|
||||
let target_port = rule.target_port;
|
||||
let status: SharedStatus = Arc::new(Mutex::new(ForwardStatus::Listening));
|
||||
let task_status = status.clone();
|
||||
let handle = tokio::spawn(async move {
|
||||
loop {
|
||||
let exit = loop {
|
||||
let sock = match accept_retrying(&listener).await {
|
||||
Some((sock, _peer)) => sock,
|
||||
None => break,
|
||||
None => break LoopExit::ListenerLost,
|
||||
};
|
||||
if !conn.is_alive() {
|
||||
break;
|
||||
break LoopExit::ConnectionLost;
|
||||
}
|
||||
let conn = conn.clone();
|
||||
let target_host = target_host.clone();
|
||||
@@ -432,27 +474,21 @@ impl SshForwardRegistry {
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
};
|
||||
*task_status.lock().unwrap() = loop_exit_status(exit);
|
||||
});
|
||||
(bound, ForwardStatus::Listening, ForwardCancel::Task(handle))
|
||||
(bound, status, ForwardCancel::Task(handle))
|
||||
}
|
||||
|
||||
async fn start_dynamic(
|
||||
&self,
|
||||
conn: &Arc<SshConnection>,
|
||||
rule: &SshForwardRule,
|
||||
) -> (u16, ForwardStatus, ForwardCancel) {
|
||||
) -> (u16, SharedStatus, ForwardCancel) {
|
||||
let listener = match TcpListener::bind((rule.bind_host.as_str(), rule.bind_port)).await {
|
||||
Ok(l) => l,
|
||||
Err(e) => {
|
||||
return (
|
||||
rule.bind_port,
|
||||
ForwardStatus::Error(format!(
|
||||
"bind {}:{} failed: {e}",
|
||||
rule.bind_host, rule.bind_port
|
||||
)),
|
||||
ForwardCancel::None,
|
||||
);
|
||||
return (rule.bind_port, bind_failed(rule, e), ForwardCancel::None);
|
||||
}
|
||||
};
|
||||
let bound = listener
|
||||
@@ -460,14 +496,16 @@ impl SshForwardRegistry {
|
||||
.map(|a| a.port())
|
||||
.unwrap_or(rule.bind_port);
|
||||
let conn = conn.clone();
|
||||
let status: SharedStatus = Arc::new(Mutex::new(ForwardStatus::Listening));
|
||||
let task_status = status.clone();
|
||||
let handle = tokio::spawn(async move {
|
||||
loop {
|
||||
let exit = loop {
|
||||
let sock = match accept_retrying(&listener).await {
|
||||
Some((sock, _peer)) => sock,
|
||||
None => break,
|
||||
None => break LoopExit::ListenerLost,
|
||||
};
|
||||
if !conn.is_alive() {
|
||||
break;
|
||||
break LoopExit::ConnectionLost;
|
||||
}
|
||||
let conn = conn.clone();
|
||||
tokio::spawn(async move {
|
||||
@@ -492,16 +530,21 @@ impl SshForwardRegistry {
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
};
|
||||
*task_status.lock().unwrap() = loop_exit_status(exit);
|
||||
});
|
||||
(bound, ForwardStatus::Listening, ForwardCancel::Task(handle))
|
||||
(bound, status, ForwardCancel::Task(handle))
|
||||
}
|
||||
|
||||
/// Unlike the two above, a remote forward has no accept loop of its own to
|
||||
/// notice a dead transport: the far end opens the channels and russh hands
|
||||
/// them to the connection's handler. Its status stays whatever the
|
||||
/// `tcpip-forward` request answered.
|
||||
async fn start_remote(
|
||||
&self,
|
||||
conn: &Arc<SshConnection>,
|
||||
rule: &SshForwardRule,
|
||||
) -> (u16, ForwardStatus, ForwardCancel) {
|
||||
) -> (u16, SharedStatus, ForwardCancel) {
|
||||
match conn
|
||||
.add_remote_forward(
|
||||
&rule.bind_host,
|
||||
@@ -513,7 +556,7 @@ impl SshForwardRegistry {
|
||||
{
|
||||
Ok(bound) => (
|
||||
bound,
|
||||
ForwardStatus::Listening,
|
||||
Arc::new(Mutex::new(ForwardStatus::Listening)),
|
||||
ForwardCancel::Remote {
|
||||
conn: Arc::downgrade(conn),
|
||||
bind_host: rule.bind_host.clone(),
|
||||
@@ -522,7 +565,9 @@ impl SshForwardRegistry {
|
||||
),
|
||||
Err(e) => (
|
||||
rule.bind_port,
|
||||
ForwardStatus::Error(format!("remote forward request denied: {e}")),
|
||||
Arc::new(Mutex::new(ForwardStatus::Error(format!(
|
||||
"remote forward request denied: {e}"
|
||||
)))),
|
||||
ForwardCancel::None,
|
||||
),
|
||||
}
|
||||
@@ -576,7 +621,7 @@ impl SshForwardRegistry {
|
||||
};
|
||||
let id = self.next_id.fetch_add(1, Ordering::Relaxed);
|
||||
let (bind_port, status, cancel) = self.start_local(&conn, &rule).await;
|
||||
if let ForwardStatus::Error(e) = &status {
|
||||
if let ForwardStatus::Error(e) = &*status.lock().unwrap() {
|
||||
return Err(io::Error::other(e.clone()));
|
||||
}
|
||||
let entry = ForwardEntry {
|
||||
@@ -617,7 +662,9 @@ impl SshForwardRegistry {
|
||||
&& e.kind == SshForwardKind::Local
|
||||
&& e.target_host == remote_host
|
||||
&& e.target_port == remote_port
|
||||
&& matches!(e.status, ForwardStatus::Listening)
|
||||
// Now that a dead loop says so, this stops handing out the
|
||||
// port of a forward that no longer serves anything.
|
||||
&& matches!(*e.status.lock().unwrap(), ForwardStatus::Listening)
|
||||
})
|
||||
.map(|e| e.bind_port)
|
||||
}
|
||||
@@ -814,6 +861,47 @@ mod tests {
|
||||
assert_eq!(table.lookup("localhost", 9000), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_stopped_forward_says_why_it_stopped() {
|
||||
let listener = loop_exit_status(LoopExit::ListenerLost);
|
||||
let connection = loop_exit_status(LoopExit::ConnectionLost);
|
||||
assert_ne!(
|
||||
listener, connection,
|
||||
"a socket that closed and a transport that died are different problems"
|
||||
);
|
||||
for status in [&listener, &connection] {
|
||||
assert!(
|
||||
matches!(status, ForwardStatus::Error(_)),
|
||||
"an older remote has to be able to deserialise this, so no new variant"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A forward's status used to be copied into `ManagedForward` from a field
|
||||
/// written once when the forward was set up, so a forward whose loop had
|
||||
/// long since exited answered `Listening` to every poll of the panel.
|
||||
#[tokio::test]
|
||||
async fn a_forward_whose_loop_exited_stops_reporting_listening() {
|
||||
let reg = SshForwardRegistry::default();
|
||||
push_listener(®, ForwardOwner::Pane(7), 0).await;
|
||||
assert!(matches!(reg.list(7)[0].status, ForwardStatus::Listening));
|
||||
|
||||
// What the accept loop does on its way out. The entry is already in the
|
||||
// registry by then, which is the whole reason the status is shared.
|
||||
let status = reg.owners.lock().unwrap()[&ForwardOwner::Pane(7)][0]
|
||||
.status
|
||||
.clone();
|
||||
*status.lock().unwrap() = loop_exit_status(LoopExit::ConnectionLost);
|
||||
|
||||
match ®.list(7)[0].status {
|
||||
ForwardStatus::Error(msg) => assert!(
|
||||
msg.contains("SSH connection"),
|
||||
"the panel needs a reason, not just a red badge: {msg}"
|
||||
),
|
||||
other => panic!("a dead forward still reports {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn registry_add_list_remove_teardown_bookkeeping() {
|
||||
let reg = SshForwardRegistry::default();
|
||||
@@ -827,7 +915,7 @@ mod tests {
|
||||
target_host: "h".into(),
|
||||
target_port: 80,
|
||||
description: None,
|
||||
status: ForwardStatus::Listening,
|
||||
status: Arc::new(Mutex::new(ForwardStatus::Listening)),
|
||||
cancel: ForwardCancel::Task(task),
|
||||
auto_local: false,
|
||||
}
|
||||
@@ -872,7 +960,7 @@ mod tests {
|
||||
target_host: "h".into(),
|
||||
target_port: 80,
|
||||
description: None,
|
||||
status: ForwardStatus::Listening,
|
||||
status: Arc::new(Mutex::new(ForwardStatus::Listening)),
|
||||
cancel: ForwardCancel::Task(handle),
|
||||
auto_local: false,
|
||||
}
|
||||
@@ -933,7 +1021,7 @@ mod tests {
|
||||
target_host: "127.0.0.1".into(),
|
||||
target_port: 3000,
|
||||
description: None,
|
||||
status: ForwardStatus::Listening,
|
||||
status: Arc::new(Mutex::new(ForwardStatus::Listening)),
|
||||
cancel: ForwardCancel::Task(handle),
|
||||
auto_local: true,
|
||||
};
|
||||
|
||||
+10
-8
@@ -1478,18 +1478,20 @@ impl RemoteTerminal {
|
||||
query(job_id).unwrap_or_default()
|
||||
}
|
||||
|
||||
pub fn sftp_transfer_list(pane_id: u64) -> Vec<SftpJobProgress> {
|
||||
fn query(pane_id: u64) -> anyhow::Result<Vec<SftpJobProgress>> {
|
||||
/// A failed poll is not an empty transfer list: the caller has to be able
|
||||
/// to keep the jobs it already knows about, so this reports the failure
|
||||
/// the way `sftp_list` does rather than answering with an empty `Vec`.
|
||||
pub fn sftp_transfer_list(pane_id: u64) -> Result<Vec<SftpJobProgress>, String> {
|
||||
fn query(pane_id: u64) -> anyhow::Result<Result<Vec<SftpJobProgress>, String>> {
|
||||
let mut stream = connect()?;
|
||||
ClientMsg::SftpTransferList { pane_id }.encode(&mut stream)?;
|
||||
match DaemonMsg::read(&mut stream)? {
|
||||
Ok(match DaemonMsg::read(&mut stream)? {
|
||||
DaemonMsg::SftpTransferProgress(jobs) => Ok(jobs),
|
||||
other => Err(anyhow::anyhow!(
|
||||
"unexpected reply to SftpTransferList: {other:?}"
|
||||
)),
|
||||
}
|
||||
DaemonMsg::Error(msg) => Err(msg),
|
||||
other => Err(format!("unexpected reply to SftpTransferList: {other:?}")),
|
||||
})
|
||||
}
|
||||
query(pane_id).unwrap_or_default()
|
||||
query(pane_id).unwrap_or_else(|e| Err(e.to_string()))
|
||||
}
|
||||
|
||||
pub fn add_forward(pane_id: u64, rule: SshForwardRule) -> Vec<ManagedForward> {
|
||||
|
||||
@@ -2654,6 +2654,9 @@ impl TerminalView {
|
||||
cx.background_spawn(async move { route.transfer_list() })
|
||||
.await
|
||||
};
|
||||
// A poll that failed says nothing about the job — keep asking
|
||||
// until it answers or the budget above runs out.
|
||||
let Ok(listed) = listed else { continue };
|
||||
let Some(progress) = listed.into_iter().find(|j| j.job_id == job) else {
|
||||
// Pruned after the retention window, or the daemon restarted:
|
||||
// there is nothing left to report either way.
|
||||
|
||||
+139
-39
@@ -66,6 +66,9 @@ pub(crate) enum TreeNote {
|
||||
/// The search stopped at `SEARCH_LIMIT`; the list is a prefix, not the
|
||||
/// whole answer, and has to say so.
|
||||
SearchCapped,
|
||||
/// The search never ran to an answer — the host refused it or the link to
|
||||
/// it went away. An empty list here means nothing at all.
|
||||
SearchFailed,
|
||||
}
|
||||
|
||||
/// `landed` is how many entries the listing returned, or `None` when nothing
|
||||
@@ -122,6 +125,10 @@ struct SearchState {
|
||||
pending: String,
|
||||
hidden: bool,
|
||||
hits: Vec<TreeEntry>,
|
||||
/// Whether the last search came back as a failure rather than as no hits.
|
||||
/// The two used to print the same "Nothing matches …", which is the same
|
||||
/// lie `unreadable` was added to stop a directory listing from telling.
|
||||
failed: bool,
|
||||
}
|
||||
|
||||
impl SearchState {
|
||||
@@ -134,22 +141,25 @@ impl SearchState {
|
||||
self.hidden = show_hidden;
|
||||
if query.is_empty() {
|
||||
self.hits.clear();
|
||||
self.failed = false;
|
||||
return None;
|
||||
}
|
||||
Some(self.generation)
|
||||
}
|
||||
|
||||
fn accept(&mut self, generation: u64, hits: Vec<TreeEntry>) -> bool {
|
||||
fn accept(&mut self, generation: u64, ok: bool, hits: Vec<TreeEntry>) -> bool {
|
||||
if self.generation != generation {
|
||||
return false;
|
||||
}
|
||||
self.hits = hits;
|
||||
self.failed = !ok;
|
||||
true
|
||||
}
|
||||
|
||||
fn restart(&mut self) {
|
||||
self.generation += 1;
|
||||
self.pending.clear();
|
||||
self.failed = false;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -438,7 +448,14 @@ impl FileTreeState {
|
||||
host,
|
||||
cx,
|
||||
move |h| {
|
||||
h.search(&roots, &query, SEARCH_LIMIT, SEARCH_MAX_DIRS, show_hidden)
|
||||
// `(ok, hits)` the way `spawn_load` reports a listing:
|
||||
// a search the host refused is not a search with no
|
||||
// hits, and the column has to be able to tell them
|
||||
// apart before it says "Nothing matches".
|
||||
let found =
|
||||
h.search(&roots, &query, SEARCH_LIMIT, SEARCH_MAX_DIRS, show_hidden);
|
||||
let ok = found.is_ok();
|
||||
let hits = found
|
||||
.unwrap_or_default()
|
||||
.into_iter()
|
||||
.map(|hit| TreeEntry {
|
||||
@@ -447,10 +464,11 @@ impl FileTreeState {
|
||||
is_dir: hit.is_dir,
|
||||
ignored: hit.ignored,
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.collect::<Vec<_>>();
|
||||
(ok, hits)
|
||||
},
|
||||
move |app, hits, cx| {
|
||||
if app.file_tree.search.accept(generation, hits) {
|
||||
move |app, (ok, hits), cx| {
|
||||
if app.file_tree.search.accept(generation, ok, hits) {
|
||||
cx.notify();
|
||||
}
|
||||
},
|
||||
@@ -461,35 +479,51 @@ impl FileTreeState {
|
||||
}
|
||||
|
||||
fn search_rows(&self) -> Vec<TreeRow> {
|
||||
let mut rows: Vec<TreeRow> = self
|
||||
.search
|
||||
.hits
|
||||
.iter()
|
||||
.map(|e| TreeRow {
|
||||
entry: e.clone(),
|
||||
depth: 0,
|
||||
is_root: false,
|
||||
expanded: false,
|
||||
note: None,
|
||||
})
|
||||
.collect();
|
||||
if rows.len() >= SEARCH_LIMIT {
|
||||
rows.push(TreeRow {
|
||||
entry: TreeEntry {
|
||||
name: String::new(),
|
||||
path: PathBuf::new(),
|
||||
is_dir: false,
|
||||
ignored: false,
|
||||
},
|
||||
depth: 0,
|
||||
is_root: false,
|
||||
expanded: false,
|
||||
note: Some(TreeNote::SearchCapped),
|
||||
});
|
||||
}
|
||||
rows
|
||||
search_rows(&self.search)
|
||||
}
|
||||
}
|
||||
|
||||
/// The rows a search puts in the column, and the note that stands for whatever
|
||||
/// they do not say by themselves.
|
||||
fn search_rows(search: &SearchState) -> Vec<TreeRow> {
|
||||
let mut rows: Vec<TreeRow> = search
|
||||
.hits
|
||||
.iter()
|
||||
.map(|e| TreeRow {
|
||||
entry: e.clone(),
|
||||
depth: 0,
|
||||
is_root: false,
|
||||
expanded: false,
|
||||
note: None,
|
||||
})
|
||||
.collect();
|
||||
let note = if search.failed {
|
||||
// Ahead of the cap: a failed search has no hits to have capped, and
|
||||
// this is the one thing worth saying about it.
|
||||
Some(TreeNote::SearchFailed)
|
||||
} else if rows.len() >= SEARCH_LIMIT {
|
||||
Some(TreeNote::SearchCapped)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
if let Some(note) = note {
|
||||
rows.push(TreeRow {
|
||||
entry: TreeEntry {
|
||||
name: String::new(),
|
||||
path: PathBuf::new(),
|
||||
is_dir: false,
|
||||
ignored: false,
|
||||
},
|
||||
depth: 0,
|
||||
is_root: false,
|
||||
expanded: false,
|
||||
note: Some(note),
|
||||
});
|
||||
}
|
||||
rows
|
||||
}
|
||||
|
||||
impl FileTreeState {
|
||||
pub(crate) fn visible_rows(
|
||||
&self,
|
||||
host: HostId,
|
||||
@@ -1116,6 +1150,20 @@ impl Tty7App {
|
||||
}
|
||||
|
||||
let target = new_path.clone();
|
||||
// The same gap `file_tree_delete` closed: on its own, "Permission
|
||||
// denied (os error 13)" says neither which file nor what was being
|
||||
// done to it, and those are the only two things worth knowing here.
|
||||
// A rename is named by the name it is leaving, which is the one still
|
||||
// on screen to find.
|
||||
let (context, failed_name) = match &edit {
|
||||
TreeEdit::Rename { path, .. } => (
|
||||
L10nKey::FileTreeRenameFailed,
|
||||
path.file_name()
|
||||
.map(|n| n.to_string_lossy().to_string())
|
||||
.unwrap_or_else(|| name.clone()),
|
||||
),
|
||||
_ => (L10nKey::FileTreeCreateFailed, name.clone()),
|
||||
};
|
||||
HostOps::run_in(
|
||||
host,
|
||||
window,
|
||||
@@ -1141,8 +1189,12 @@ impl Tty7App {
|
||||
{
|
||||
code.selected = None;
|
||||
}
|
||||
use gpui_component::WindowExt as _;
|
||||
window.push_notification(format!("{e}"), cx);
|
||||
HostOps::notify_err(
|
||||
window,
|
||||
cx,
|
||||
&t_fmt(context, &[("name", &failed_name)]),
|
||||
&e,
|
||||
);
|
||||
}
|
||||
}
|
||||
cx.notify();
|
||||
@@ -1503,6 +1555,7 @@ impl Tty7App {
|
||||
TreeNote::HiddenOnly => (L10nKey::TreeDirHiddenOnly, muted),
|
||||
TreeNote::Unreadable => (L10nKey::TreeDirUnreadable, cx.theme().danger),
|
||||
TreeNote::SearchCapped => (L10nKey::TreeSearchCapped, muted),
|
||||
TreeNote::SearchFailed => (L10nKey::TreeSearchFailed, cx.theme().danger),
|
||||
};
|
||||
return vec![
|
||||
h_flex()
|
||||
@@ -1519,9 +1572,10 @@ impl Tty7App {
|
||||
TreeNote::SearchCapped => t_fmt(key, &[("n", &SEARCH_LIMIT.to_string())]),
|
||||
_ => t(key).to_string(),
|
||||
})
|
||||
// Every note but the capped-search one stands for a real
|
||||
// directory, and carries its path; that one stands for the
|
||||
// rest of a search and has nowhere to put anything.
|
||||
// Every note but the two search ones stands for a real
|
||||
// directory, and carries its path; those stand for the rest
|
||||
// of a search, or for one that never ran, and have nowhere
|
||||
// to put anything.
|
||||
.when(!path.as_os_str().is_empty(), |d| {
|
||||
d.drag_over::<ExternalPaths>(|s, _, _, cx| {
|
||||
s.bg(cx.theme().drag_border.opacity(0.14))
|
||||
@@ -2058,6 +2112,28 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_failed_search_says_so_instead_of_drawing_no_rows() {
|
||||
let mut search = SearchState::default();
|
||||
let walk = search.retarget("foo", false).expect("a new query walks");
|
||||
search.accept(walk, false, Vec::new());
|
||||
assert_eq!(
|
||||
search_rows(&search)
|
||||
.iter()
|
||||
.filter_map(|r| r.note)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![TreeNote::SearchFailed],
|
||||
"without a note the column falls through to \"Nothing matches\""
|
||||
);
|
||||
|
||||
// A search that ran and found nothing still draws nothing.
|
||||
let walk = search
|
||||
.retarget("bar", false)
|
||||
.expect("a changed query walks");
|
||||
search.accept(walk, true, Vec::new());
|
||||
assert!(search_rows(&search).is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_listing_superseded_in_flight_is_still_shown() {
|
||||
let mut loads: InFlight<DirKey> = InFlight::default();
|
||||
@@ -2426,10 +2502,10 @@ mod tests {
|
||||
assert_ne!(first, second);
|
||||
|
||||
assert!(
|
||||
!search.accept(first, vec![entry("stale.rs", false)]),
|
||||
!search.accept(first, true, vec![entry("stale.rs", false)]),
|
||||
"the overtaken walk's hits are dropped"
|
||||
);
|
||||
assert!(search.accept(second, vec![entry("foo.rs", false)]));
|
||||
assert!(search.accept(second, true, vec![entry("foo.rs", false)]));
|
||||
assert_eq!(search.hits.len(), 1);
|
||||
|
||||
let third = search
|
||||
@@ -2445,6 +2521,30 @@ mod tests {
|
||||
assert!(search.retarget("foo", true).is_some(), "restart re-walks");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_search_that_failed_is_not_a_search_with_no_hits() {
|
||||
let mut search = SearchState::default();
|
||||
let walk = search.retarget("foo", false).expect("a new query walks");
|
||||
assert!(search.accept(walk, false, Vec::new()));
|
||||
assert!(
|
||||
search.failed,
|
||||
"an empty list from a host that refused the walk is not an answer"
|
||||
);
|
||||
|
||||
// And it is not carried past the query it belongs to.
|
||||
let next = search
|
||||
.retarget("food", false)
|
||||
.expect("a changed query walks");
|
||||
assert!(search.accept(next, true, vec![entry("food.rs", false)]));
|
||||
assert!(!search.failed);
|
||||
|
||||
let last = search.retarget("foodie", false).expect("and again");
|
||||
assert!(search.accept(last, false, Vec::new()));
|
||||
assert!(search.failed);
|
||||
search.retarget("", false);
|
||||
assert!(!search.failed, "an emptied box has nothing to report");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_tree_reads_the_same_listing_out_of_the_host() {
|
||||
let host = tty7_core::host::local::LocalHost::new();
|
||||
|
||||
@@ -39,6 +39,7 @@ pub fn translate_en(key: L10nKey) -> &'static str {
|
||||
L10nKey::TreeDirHiddenOnly => "Only hidden files",
|
||||
L10nKey::TreeDirUnreadable => "Could not be read",
|
||||
L10nKey::TreeSearchCapped => "First {n} matches",
|
||||
L10nKey::TreeSearchFailed => "Search failed",
|
||||
L10nKey::FileChangedOnDisk => "File changed on disk",
|
||||
L10nKey::Reload => "Reload",
|
||||
L10nKey::KeepMine => "Keep mine",
|
||||
@@ -816,6 +817,7 @@ pub fn translate_en(key: L10nKey) -> &'static str {
|
||||
L10nKey::SftpTransferDone => "done",
|
||||
L10nKey::SftpTransferCancelled => "cancelled",
|
||||
L10nKey::SftpTransferError => "error",
|
||||
L10nKey::SftpTransferListFailed => "Could not check transfers: {error}",
|
||||
L10nKey::SftpImagePasteUploadFailed => {
|
||||
"Could not upload the pasted image to {host}: {error}"
|
||||
}
|
||||
@@ -845,6 +847,8 @@ pub fn translate_en(key: L10nKey) -> &'static str {
|
||||
"The file will be deleted on {host}. There is no trash on the far side."
|
||||
}
|
||||
L10nKey::FileTreeDeleteFailed => "Could not delete {name}",
|
||||
L10nKey::FileTreeCreateFailed => "Could not create {name}",
|
||||
L10nKey::FileTreeRenameFailed => "Could not rename {name}",
|
||||
L10nKey::FileTreeContextOpen => "Open",
|
||||
L10nKey::FileTreeContextCdHere => "cd Here",
|
||||
L10nKey::FileTreeContextInsertPath => "Insert Path in Terminal",
|
||||
|
||||
@@ -39,6 +39,7 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> {
|
||||
L10nKey::TreeDirHiddenOnly => "隠しファイルのみ",
|
||||
L10nKey::TreeDirUnreadable => "読み取れません",
|
||||
L10nKey::TreeSearchCapped => "最初の {n} 件のみ",
|
||||
L10nKey::TreeSearchFailed => "検索に失敗しました",
|
||||
L10nKey::FileChangedOnDisk => "ディスク上でファイルが変更されました",
|
||||
L10nKey::Reload => "再読み込み",
|
||||
L10nKey::KeepMine => "自分の変更を保持",
|
||||
@@ -858,6 +859,7 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> {
|
||||
L10nKey::SftpTransferDone => "完了",
|
||||
L10nKey::SftpTransferCancelled => "キャンセル済み",
|
||||
L10nKey::SftpTransferError => "エラー",
|
||||
L10nKey::SftpTransferListFailed => "転送状況を取得できませんでした: {error}",
|
||||
L10nKey::SftpImagePasteUploadFailed => {
|
||||
"貼り付けた画像を {host} にアップロードできませんでした: {error}"
|
||||
}
|
||||
@@ -887,6 +889,8 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> {
|
||||
"{host} 上でファイルが削除されます。リモート側にゴミ箱はありません。"
|
||||
}
|
||||
L10nKey::FileTreeDeleteFailed => "{name} を削除できませんでした",
|
||||
L10nKey::FileTreeCreateFailed => "{name} を作成できませんでした",
|
||||
L10nKey::FileTreeRenameFailed => "{name} の名前を変更できませんでした",
|
||||
L10nKey::FileTreeContextOpen => "開く",
|
||||
L10nKey::FileTreeContextCdHere => "ここで cd",
|
||||
L10nKey::FileTreeContextInsertPath => "ターミナルにパスを挿入",
|
||||
|
||||
@@ -107,6 +107,7 @@ l10n_keys! {
|
||||
TreeDirHiddenOnly,
|
||||
TreeDirUnreadable,
|
||||
TreeSearchCapped,
|
||||
TreeSearchFailed,
|
||||
FileChangedOnDisk,
|
||||
Reload,
|
||||
KeepMine,
|
||||
@@ -624,6 +625,7 @@ l10n_keys! {
|
||||
SftpTransferDone,
|
||||
SftpTransferCancelled,
|
||||
SftpTransferError,
|
||||
SftpTransferListFailed,
|
||||
SftpImagePasteUploadFailed,
|
||||
ForwardPanelTitle,
|
||||
ForwardDisconnected,
|
||||
@@ -647,6 +649,8 @@ l10n_keys! {
|
||||
SftpDeleteFolderBody,
|
||||
SftpDeleteFileBody,
|
||||
FileTreeDeleteFailed,
|
||||
FileTreeCreateFailed,
|
||||
FileTreeRenameFailed,
|
||||
FileTreeContextOpen,
|
||||
FileTreeContextCdHere,
|
||||
FileTreeContextInsertPath,
|
||||
|
||||
@@ -39,6 +39,7 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> {
|
||||
L10nKey::TreeDirHiddenOnly => "只有隐藏文件",
|
||||
L10nKey::TreeDirUnreadable => "无法读取",
|
||||
L10nKey::TreeSearchCapped => "只显示前 {n} 个匹配",
|
||||
L10nKey::TreeSearchFailed => "搜索失败",
|
||||
L10nKey::FileChangedOnDisk => "文件在磁盘上已被修改",
|
||||
L10nKey::Reload => "重新加载",
|
||||
L10nKey::KeepMine => "保留我的版本",
|
||||
@@ -785,6 +786,7 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> {
|
||||
L10nKey::SftpTransferDone => "完成",
|
||||
L10nKey::SftpTransferCancelled => "已取消",
|
||||
L10nKey::SftpTransferError => "错误",
|
||||
L10nKey::SftpTransferListFailed => "无法获取传输状态:{error}",
|
||||
L10nKey::SftpImagePasteUploadFailed => "无法将粘贴的图片上传到 {host}:{error}",
|
||||
L10nKey::ForwardPanelTitle => "端口转发",
|
||||
L10nKey::ForwardDisconnected => "已断开",
|
||||
@@ -810,6 +812,8 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> {
|
||||
}
|
||||
L10nKey::SftpDeleteFileBody => "该文件将在 {host} 上被删除。远端没有回收站。",
|
||||
L10nKey::FileTreeDeleteFailed => "无法删除 {name}",
|
||||
L10nKey::FileTreeCreateFailed => "无法创建 {name}",
|
||||
L10nKey::FileTreeRenameFailed => "无法重命名 {name}",
|
||||
L10nKey::FileTreeContextOpen => "打开",
|
||||
L10nKey::FileTreeContextCdHere => "cd 到此处",
|
||||
L10nKey::FileTreeContextInsertPath => "在终端中插入路径",
|
||||
|
||||
+106
-18
@@ -115,21 +115,18 @@ impl SftpRoute {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn transfer_list(&self) -> Vec<SftpJobProgress> {
|
||||
pub(crate) fn transfer_list(&self) -> Result<Vec<SftpJobProgress>, String> {
|
||||
let Some(req) = self.workspace_op(crate::daemon::protocol::WorkspaceOp::SftpTransferList)
|
||||
else {
|
||||
return RemoteTerminal::sftp_transfer_list(self.pane_id);
|
||||
};
|
||||
match RemoteTerminal::on_workspace(req) {
|
||||
Ok(crate::daemon::protocol::DaemonMsg::SftpTransferProgress(jobs)) => jobs,
|
||||
Ok(other) => {
|
||||
log::warn!("unexpected reply to a workspace transfer list: {other:?}");
|
||||
Vec::new()
|
||||
}
|
||||
Err(e) => {
|
||||
log::warn!("workspace transfer list failed: {e}");
|
||||
Vec::new()
|
||||
}
|
||||
Ok(crate::daemon::protocol::DaemonMsg::SftpTransferProgress(jobs)) => Ok(jobs),
|
||||
Ok(other) => Err(t_fmt(
|
||||
L10nKey::SftpErrorUnexpectedReply,
|
||||
&[("reply", &format!("{other:?}"))],
|
||||
)),
|
||||
Err(e) => Err(e.to_string()),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -143,6 +140,12 @@ pub(crate) struct SftpPanelState {
|
||||
pub(crate) filter_input: gpui::Entity<InputState>,
|
||||
pub(crate) error: Option<String>,
|
||||
pub(crate) jobs: Vec<SftpJobProgress>,
|
||||
/// Why the last transfer poll came back empty-handed, if it did.
|
||||
///
|
||||
/// Kept apart from `error`, which blanks the directory listing: a poll
|
||||
/// that could not reach the daemon says nothing about the listing already
|
||||
/// on screen, and the transfer tray is the only place it belongs.
|
||||
jobs_error: Option<String>,
|
||||
/// Uploads this panel started whose landing it has not listed yet.
|
||||
///
|
||||
/// An upload is written to `<name>.tty7-upload-<hex>` and renamed into
|
||||
@@ -188,6 +191,7 @@ impl SftpPanelState {
|
||||
filter_input,
|
||||
error: None,
|
||||
jobs: Vec::new(),
|
||||
jobs_error: None,
|
||||
uploads_awaiting_listing: HashSet::new(),
|
||||
claimed_downloads: HashSet::new(),
|
||||
dismissed_jobs: HashSet::new(),
|
||||
@@ -223,6 +227,24 @@ fn uploads_still_running(owed: &HashSet<u64>, jobs: &[SftpJobProgress]) -> HashS
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// What the tray shows after a poll: the jobs to draw, and the failure to say
|
||||
/// out loud beside them.
|
||||
///
|
||||
/// A poll that failed used to come back as an empty `Vec`, which reads as "the
|
||||
/// transfers are all gone" — the tray disappeared and every upload the panel
|
||||
/// was waiting on counted as landed. Over a link that is down that is not a
|
||||
/// blink but the permanent answer, so a failure keeps the previous list and is
|
||||
/// reported instead of replacing it.
|
||||
fn apply_poll(
|
||||
previous: Vec<SftpJobProgress>,
|
||||
reply: Result<Vec<SftpJobProgress>, String>,
|
||||
) -> (Vec<SftpJobProgress>, Option<String>) {
|
||||
match reply {
|
||||
Ok(jobs) => (jobs, None),
|
||||
Err(e) => (previous, Some(e)),
|
||||
}
|
||||
}
|
||||
|
||||
fn is_dir_like(e: &SftpEntry) -> bool {
|
||||
matches!(e.kind, SftpEntryKind::Dir)
|
||||
|| (matches!(e.kind, SftpEntryKind::Symlink) && e.target_is_dir)
|
||||
@@ -362,6 +384,7 @@ impl Tty7App {
|
||||
self.sftp_panel.editing_path = None;
|
||||
self.sftp_panel.editing_path_sub.clear();
|
||||
self.sftp_panel.jobs.clear();
|
||||
self.sftp_panel.jobs_error = None;
|
||||
self.sftp_panel.open_workspace = None;
|
||||
self.sftp_panel.poll_gen = self.sftp_panel.poll_gen.wrapping_add(1);
|
||||
cx.notify();
|
||||
@@ -1007,12 +1030,28 @@ impl Tty7App {
|
||||
/// listing on screen was the one taken while the temporary name existed —
|
||||
/// and it stayed, so a finished upload read as a file with a hash glued to
|
||||
/// its name.
|
||||
fn sftp_apply_jobs(&mut self, jobs: Vec<SftpJobProgress>, cx: &mut Context<Self>) {
|
||||
///
|
||||
/// A poll that failed is not a job list, so it settles nothing: the uploads
|
||||
/// still owe their listing, and asking for one now would only refresh from
|
||||
/// the same unreachable daemon.
|
||||
fn sftp_apply_jobs(
|
||||
&mut self,
|
||||
reply: Result<Vec<SftpJobProgress>, String>,
|
||||
cx: &mut Context<Self>,
|
||||
) {
|
||||
let previous = std::mem::take(&mut self.sftp_panel.jobs);
|
||||
let (jobs, failure) = apply_poll(previous, reply);
|
||||
let failed = failure.is_some();
|
||||
self.sftp_panel.jobs = jobs;
|
||||
self.sftp_panel.jobs_error = failure;
|
||||
if failed {
|
||||
cx.notify();
|
||||
return;
|
||||
}
|
||||
let owed = &self.sftp_panel.uploads_awaiting_listing;
|
||||
let still_running = uploads_still_running(owed, &jobs);
|
||||
let still_running = uploads_still_running(owed, &self.sftp_panel.jobs);
|
||||
let settled = still_running.len() != owed.len();
|
||||
self.sftp_panel.uploads_awaiting_listing = still_running;
|
||||
self.sftp_panel.jobs = jobs;
|
||||
cx.notify();
|
||||
if settled {
|
||||
self.sftp_refresh(cx);
|
||||
@@ -1568,7 +1607,11 @@ impl Tty7App {
|
||||
.iter()
|
||||
.filter(|j| history || !self.sftp_panel.dismissed_jobs.contains(&j.job_id))
|
||||
.collect();
|
||||
if jobs.is_empty() && !history {
|
||||
// A poll that failed is worth a tray of its own. Without one the whole
|
||||
// footer vanishes at the moment the panel stops being able to say
|
||||
// anything about the transfers, which reads as "they are all finished".
|
||||
let jobs_error = self.sftp_panel.jobs_error.as_ref();
|
||||
if jobs.is_empty() && !history && jobs_error.is_none() {
|
||||
return None;
|
||||
}
|
||||
|
||||
@@ -1596,7 +1639,12 @@ impl Tty7App {
|
||||
} else {
|
||||
0.0
|
||||
};
|
||||
let summary = if running > 0 {
|
||||
// The failed poll outranks the counts, because the counts are only as
|
||||
// fresh as the last poll that got through and the summary is the one
|
||||
// line a collapsed tray gets to say.
|
||||
let summary = if let Some(e) = jobs_error {
|
||||
t_fmt(L10nKey::SftpTransferListFailed, &[("error", e)])
|
||||
} else if running > 0 {
|
||||
t_fmt(
|
||||
L10nKey::SftpTransferSummaryRunning,
|
||||
&[
|
||||
@@ -1612,7 +1660,7 @@ impl Tty7App {
|
||||
} else {
|
||||
t(L10nKey::SftpTransferSummaryIdle).to_string()
|
||||
};
|
||||
let summary_color = if running == 0 && failed > 0 {
|
||||
let summary_color = if jobs_error.is_some() || (running == 0 && failed > 0) {
|
||||
danger
|
||||
} else {
|
||||
muted
|
||||
@@ -1671,13 +1719,23 @@ impl Tty7App {
|
||||
|
||||
let body = expanded.then(|| {
|
||||
let inner: Div = if jobs.is_empty() {
|
||||
// The summary above says the same thing when a poll failed, but
|
||||
// it is a single truncated line; this one wraps, so it is where
|
||||
// the reason is actually readable.
|
||||
let (text, color): (gpui::SharedString, _) = match jobs_error {
|
||||
Some(e) => (
|
||||
t_fmt(L10nKey::SftpTransferListFailed, &[("error", e)]).into(),
|
||||
danger,
|
||||
),
|
||||
None => (t(L10nKey::SftpNoTransfers).into(), muted),
|
||||
};
|
||||
v_flex().child(
|
||||
div()
|
||||
.px(px(CONTENT_INSET))
|
||||
.py(px(3.))
|
||||
.text_size(rems(META))
|
||||
.text_color(muted)
|
||||
.child(t(L10nKey::SftpNoTransfers)),
|
||||
.text_color(color)
|
||||
.child(text),
|
||||
)
|
||||
} else {
|
||||
let mut list = v_flex().px(px(CONTENT_INSET)).pb(px(6.)).gap(px(6.));
|
||||
@@ -1883,6 +1941,36 @@ mod tests {
|
||||
assert_eq!(running, HashSet::from([8]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_failed_poll_keeps_the_transfers_it_cannot_see() {
|
||||
let previous = vec![upload(7, SftpJobState::Running)];
|
||||
let (jobs, failure) = apply_poll(previous.clone(), Err("broken pipe".into()));
|
||||
assert_eq!(jobs.len(), 1, "the last list anyone saw is still the truth");
|
||||
assert_eq!(jobs[0].job_id, 7);
|
||||
assert_eq!(failure.as_deref(), Some("broken pipe"));
|
||||
|
||||
// And the upload is still owed its listing, so nothing settles behind
|
||||
// a link that has gone quiet.
|
||||
assert_eq!(
|
||||
uploads_still_running(&HashSet::from([7]), &jobs),
|
||||
HashSet::from([7])
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_poll_that_got_through_replaces_the_list_and_clears_the_failure() {
|
||||
let previous = vec![upload(7, SftpJobState::Running)];
|
||||
let (jobs, failure) = apply_poll(previous, Ok(vec![upload(8, SftpJobState::Done)]));
|
||||
assert_eq!(jobs.len(), 1);
|
||||
assert_eq!(jobs[0].job_id, 8);
|
||||
assert!(failure.is_none());
|
||||
|
||||
// An empty reply from a daemon that answered really is an empty list.
|
||||
let (jobs, failure) = apply_poll(jobs, Ok(Vec::new()));
|
||||
assert!(jobs.is_empty());
|
||||
assert!(failure.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_second_download_is_numbered_rather_than_written_over_the_first() {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
Reference in New Issue
Block a user