mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-21 16:02:20 +00:00
fix(history): stop per-pane history from silently disabling history search
`<config>/history` was two different things at once. The window keeps the input bar's command store there as a *file* (`terminal::history`), and the daemon put each pane's `HISTFILE` in a *directory* of the same name. A path is one or the other, and the daemon created its directory the moment `per_pane_history` was switched on, so the daemon won. After that the window's `append` opened a directory, got an error, and — the call site being `if let Ok(mut f) = open(..)` — dropped the command line without a word. Up and Ctrl-R went on offering whatever had been recorded before the setting was turned on and never grew again. Nothing in the UI or the log said why, and the two halves are in different crates, so neither side had any reason to notice the other's name. Per-pane files move to `<config>/pane-history/`, which leaves the old name to the file that should have had it. An install that already tripped over this is carried across on the next start: the directory sitting at the old name is exactly what belongs at the new one, so it is renamed rather than abandoned — but only when it *is* a directory, since a plain file there is the window's store and moving that would take the very thing this repairs, and only when the new name is free, so a second call cannot bury a directory in use. Verified end to end against a daemon: with the setting on it now creates `pane-history/` 0700 with the pane files in it, and an append to `<config>/history` succeeds where it previously failed. Seeding an old-layout install with `history/pane-9` and starting the daemon leaves the line intact at the new path. Guards: one reads the window's source for the names it passes to `config_path` and fails if the daemon's directory is among them, so moving either side onto the other trips it; one covers the three ways the move can lose data (directory moves, file stays, occupied target is not overwritten). Both were checked against injected regressions. The privacy page and the integration test that read the path follow it.
This commit is contained in:
l0ng-ai
@@ -153,6 +153,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Turning on per-pane history no longer turns off history search.** The
|
||||
window keeps the input bar's own command store in the file
|
||||
`<config>/history`, and the daemon put each pane's `HISTFILE` in a
|
||||
*directory* of that same name. A path can only be one of the two, and the
|
||||
daemon created its directory the moment the setting was switched on — so from
|
||||
then on the window's append opened a directory, got an error, and dropped the
|
||||
line without a word. Up and Ctrl-R kept showing whatever had been recorded
|
||||
before the setting was turned on, and never grew again. The panes' files now
|
||||
live in `<config>/pane-history/`, and an install that already has the old
|
||||
directory is moved across on the next start, keeping its history.
|
||||
|
||||
- **A tree pull that has to be retried no longer ends with the window deleting
|
||||
the tabs it was pulling.** A window told to rebuild itself from the machine —
|
||||
a daemon back as a new process, a restart handoff that was refused, a remote
|
||||
|
||||
@@ -53,8 +53,39 @@ pub fn enabled() -> bool {
|
||||
crate::core::config::Config::load().per_pane_history
|
||||
}
|
||||
|
||||
/// Where a pane's own `HISTFILE` lives.
|
||||
///
|
||||
/// `pane-history`, not `history`: the window keeps its input bar's own command
|
||||
/// store in the *file* `<config>/history` (`terminal::history`), and this used
|
||||
/// to be that same path as a *directory*. Both cannot exist. Whichever came
|
||||
/// first won, and since the daemon creates its directory the moment
|
||||
/// `per_pane_history` is switched on, that was this one — after which the
|
||||
/// window's `append` opened a directory, failed, and dropped every command on
|
||||
/// the floor without a word. Turning on per-pane history quietly turned off
|
||||
/// history search.
|
||||
///
|
||||
/// An install that already tripped over it is carried across: the directory is
|
||||
/// exactly what belongs at the new name, and moving it frees the old one for
|
||||
/// the file that should have been there all along.
|
||||
fn dir() -> Option<PathBuf> {
|
||||
crate::core::config::config_path("history")
|
||||
let dir = crate::core::config::config_path("pane-history")?;
|
||||
let displaced = crate::core::config::config_path("history")?;
|
||||
adopt_a_displaced_directory(&displaced, &dir);
|
||||
Some(dir)
|
||||
}
|
||||
|
||||
/// Move an install that predates the rename onto the new name.
|
||||
///
|
||||
/// Only when the old path is a *directory*: a plain `history` file there is the
|
||||
/// window's own store, and moving that would take the very thing this repairs.
|
||||
/// Only when the new path is absent, so a second call cannot bury a directory
|
||||
/// that is already in use. A failed rename is not worth reporting — the caller
|
||||
/// goes on to create the new directory, and the worst case is per-pane history
|
||||
/// starting empty, which is where it starts for everyone else.
|
||||
fn adopt_a_displaced_directory(displaced: &Path, dir: &Path) {
|
||||
if !dir.exists() && displaced.is_dir() {
|
||||
let _ = std::fs::rename(displaced, dir);
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn path_for(pane: u64) -> Option<PathBuf> {
|
||||
@@ -223,7 +254,7 @@ mod tests {
|
||||
/// Where a pane's history goes is named on the privacy page.
|
||||
///
|
||||
/// With per-pane history on, a pane's `HISTFILE` is redirected into
|
||||
/// `<config>/history/`, and what lands there is the command lines someone
|
||||
/// `<config>/pane-history/`, and what lands there is the command lines someone
|
||||
/// typed — the most sensitive thing tty7 causes to be written anywhere. The
|
||||
/// page enumerates `<config>/scrollback/*.bin` down to its mode and
|
||||
/// retention; this directory belongs there on the same footing, and said
|
||||
@@ -235,14 +266,110 @@ mod tests {
|
||||
fn the_per_pane_history_location_is_on_the_privacy_page() {
|
||||
const PRIVACY: &str = include_str!("../../../../docs/reference/privacy.mdx");
|
||||
assert!(
|
||||
PRIVACY.contains("<config>/history/"),
|
||||
"per-pane history redirects HISTFILE into <config>/history/, and the \
|
||||
PRIVACY.contains("<config>/pane-history/"),
|
||||
"per-pane history redirects HISTFILE into <config>/pane-history/, and the \
|
||||
privacy page never names it"
|
||||
);
|
||||
}
|
||||
|
||||
use super::*;
|
||||
|
||||
/// A pane's history does not land on top of the window's.
|
||||
///
|
||||
/// The window keeps the input bar's command store in the *file*
|
||||
/// `<config>/history`, and this module used to put its per-pane files in a
|
||||
/// *directory* of that same name. A path is one or the other. The daemon
|
||||
/// created its directory the moment `per_pane_history` was switched on, so
|
||||
/// the daemon won; from then on the window's `append` opened a directory,
|
||||
/// got an error, and — the call site being `if let Ok(mut f) = open(..)` —
|
||||
/// dropped the line without a word. Switching on per-pane history switched
|
||||
/// off history search, and nothing said so.
|
||||
///
|
||||
/// Read from the window's source rather than restating the name here, so
|
||||
/// that moving *either* side onto the other's path fails this.
|
||||
#[test]
|
||||
fn a_panes_history_does_not_land_on_the_windows_own() {
|
||||
const WINDOW: &str = include_str!("../../../../src/terminal/history.rs");
|
||||
let names: Vec<&str> = WINDOW
|
||||
.match_indices("config_path(\"")
|
||||
.filter_map(|(i, m)| {
|
||||
let rest = &WINDOW[i + m.len()..];
|
||||
rest.find('"').map(|end| &rest[..end])
|
||||
})
|
||||
.collect();
|
||||
assert!(
|
||||
names.contains(&"history"),
|
||||
"expected the window to still keep a store called `history`; if it \
|
||||
was renamed, this guard needs to learn the new name — found {names:?}"
|
||||
);
|
||||
let ours = dir_name();
|
||||
assert!(
|
||||
!names.contains(&ours),
|
||||
"the daemon puts per-pane history at <config>/{ours}/ and the window \
|
||||
uses <config>/{ours} too ({names:?}); one is a directory and one is \
|
||||
a file, so whichever is created first makes the other fail silently"
|
||||
);
|
||||
}
|
||||
|
||||
/// The name only, so the guard above can compare without a config dir.
|
||||
fn dir_name() -> &'static str {
|
||||
"pane-history"
|
||||
}
|
||||
|
||||
/// An install that already tripped over the collision is carried across,
|
||||
/// and one that did not is left alone.
|
||||
///
|
||||
/// Three cases, because each is a different way to lose data: a directory
|
||||
/// at the old name is what belongs at the new one and moves; a *file* there
|
||||
/// is the window's store and must not be dragged along; and a new name that
|
||||
/// already exists is in use, so the old one stays put rather than being
|
||||
/// buried on top of it.
|
||||
#[test]
|
||||
fn the_old_location_moves_only_when_it_is_the_daemons_to_move() {
|
||||
let root = std::env::temp_dir().join(format!("tty7-hist-move-{}", std::process::id()));
|
||||
let case = |name: &str| {
|
||||
let d = root.join(name);
|
||||
std::fs::remove_dir_all(&d).ok();
|
||||
std::fs::create_dir_all(&d).unwrap();
|
||||
(d.join("history"), d.join("pane-history"))
|
||||
};
|
||||
|
||||
let (displaced, dir) = case("a-directory");
|
||||
std::fs::create_dir_all(&displaced).unwrap();
|
||||
std::fs::write(displaced.join("pane-7"), "ls\n").unwrap();
|
||||
adopt_a_displaced_directory(&displaced, &dir);
|
||||
assert!(!displaced.exists(), "the old directory should have moved");
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(dir.join("pane-7")).unwrap(),
|
||||
"ls\n",
|
||||
"the panes' files should have come with it"
|
||||
);
|
||||
|
||||
let (displaced, dir) = case("a-file");
|
||||
std::fs::write(&displaced, "cd /\n").unwrap();
|
||||
adopt_a_displaced_directory(&displaced, &dir);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(&displaced).unwrap(),
|
||||
"cd /\n",
|
||||
"the window's own history file is not the daemon's to move"
|
||||
);
|
||||
assert!(!dir.exists(), "and nothing should have appeared at the new name");
|
||||
|
||||
let (displaced, dir) = case("both");
|
||||
std::fs::create_dir_all(&displaced).unwrap();
|
||||
std::fs::write(displaced.join("pane-7"), "old\n").unwrap();
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
std::fs::write(dir.join("pane-7"), "in use\n").unwrap();
|
||||
adopt_a_displaced_directory(&displaced, &dir);
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(dir.join("pane-7")).unwrap(),
|
||||
"in use\n",
|
||||
"a new location already in use must not be overwritten"
|
||||
);
|
||||
|
||||
std::fs::remove_dir_all(&root).ok();
|
||||
}
|
||||
|
||||
/// The shared temp directory the rest of the suite pins, by the same name:
|
||||
/// the override is a process-wide `OnceLock`, so agreeing on the path is
|
||||
/// what keeps whoever gets there first from mattering.
|
||||
|
||||
@@ -70,7 +70,9 @@ impl Daemon {
|
||||
std::fs::read_to_string(
|
||||
self.dir
|
||||
.path()
|
||||
.join("history")
|
||||
// Not `history`: that name is the window's own command store,
|
||||
// a file. See `daemon::history::dir`.
|
||||
.join("pane-history")
|
||||
.join(format!("pane-{pane_id}")),
|
||||
)
|
||||
.ok()
|
||||
|
||||
@@ -57,7 +57,7 @@ Both of the above honour `http_proxy`. [Updates →](/reference/updates#proxies)
|
||||
| Settings, themes, window state | `~/.config/tty7/` (`%APPDATA%\tty7\` on Windows) |
|
||||
| SSH passwords and key passphrases | The **OS keychain** — never `config.json`, never plain text on disk |
|
||||
| Pane scrollback tails | `<config>/scrollback/*.bin`, mode `0600` on Unix and behind the config directory's ACL on Windows. 256 KiB per pane, kept only until something can no longer ask for it: closing a pane deletes its file at once, a restore consumes it, and a periodic pass collects the rest. |
|
||||
| Shell history | Your shell's own file, exactly as before. With **per-pane history** on, tty7 points each pane's `HISTFILE` at `<config>/history/pane-<n>` instead — the directory is created `0700`, and the files in it are written by your shell, under your umask, holding the command lines you ran in that pane. A pane's additions merge back into your usual history file when it closes. |
|
||||
| Shell history | Your shell's own file, exactly as before. With **per-pane history** on, tty7 points each pane's `HISTFILE` at `<config>/pane-history/pane-<n>` instead — the directory is created `0700`, and the files in it are written by your shell, under your umask, holding the command lines you ran in that pane. A pane's additions merge back into your usual history file when it closes. |
|
||||
| Crash reports | `<config>/crash.log`, appended whenever tty7 panics: the time, the version, the panic message and a backtrace. Capped at 256 KiB. Nothing uploads it — it is there for you to read, and to attach to an issue if you choose. |
|
||||
| Update log | `<config>/update.log`, appended by the updater while it installs a release: what it verified, what it replaced, and why it stopped if it did. Written whenever an update runs, not only under `TTY7_LOG` — the swap happens after the window is gone, so this is the only account of it. Nothing uploads it. |
|
||||
| Diagnostic log | `<config>/tty7.log`, written **only** while `TTY7_LOG` (or `RUST_LOG`) is set; unset, there is no log at all. At `info` it records the daemon and connection lifecycle. At `debug` it also carries the directories and workspace names each request names, and output from the libraries tty7 is built on — worth reading before you attach it to an issue. Capped at 4 MiB. |
|
||||
|
||||
Reference in New Issue
Block a user