diff --git a/docs/configuration.md b/docs/configuration.md index ad31aea..3a2fff5 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -131,9 +131,12 @@ than passed on a command line, so the password never shows in `ps`. **`prefix`** is the prefix the project's tracker gives its beads, written as `bd init --prefix` takes it: `prefix = "dun"` for beads named `dun-7`. `bdi` learns a prefix from the beads a tracker answers with, so this only matters -for a project it has not read. With it, a blocker -carrying the prefix is drawn as this project's bead, which was not read, and a -blocker carrying another prefix is never put down to this project. A project +for a project it has not read. With it, a blocker carrying the prefix is known +to be this project's bead. Where the directory `bdi` was started in chose what +to read, `bdi` then reads this project, draws the blocker as the bead it is, +and goes on reading the project from then on. Under `--project` it says the +blocker is this project's bead, which was not read. A blocker carrying another +prefix is never put down to this project. A project stating none may hold any blocker whose prefix no answer carries, so the line lists it among the projects not read. diff --git a/docs/design.md b/docs/design.md index 057ccd5..207c12f 100644 --- a/docs/design.md +++ b/docs/design.md @@ -451,10 +451,15 @@ that sits in an excluded project is still reported, as a pane in that project. And reloading the config while running will have to re-derive the read set from the new file, so it is kept a function of config, directory and flags rather than a value computed once at start. A dependency on another -project's bead does not reach an excluded project: it is looked for among -the projects the run reads, and one only an excluded project holds stays -work the answer does not hold. Reading an excluded tracker to draw one bead -is the unseen widening rejected above. +project's bead is looked for among the projects the run reads. Where the +directory chose the scope and a project it left out states the blocker's +prefix, the run widens to read that project, as a root on the command line +widens it, and goes on reading it from then on: the reader typed nothing the +widening overrides, and the project's line on the screen shows it was added. +Nothing else is read to find a bead, so a project nothing drawn needs stays +unread. Against a scope the reader typed, a bead only an excluded project +holds stays work the answer does not hold, because reading that tracker is +the unseen widening rejected above. ## Conventions are configuration @@ -954,8 +959,11 @@ answers carry the prefix and hold no bead by that id, or the projects that each hold one. Where no answer carries the prefix, it falls to the configured projects that gave no answer, whether refused, unreachable or left out of the run, because nothing can learn the prefix of a tracker that did not answer. -Only a project's config can state it. Where one of them states the prefix, -the line says the bead is in that project, which was not read. Otherwise it +Only a project's config can state it. Where one of them states the prefix +and the directory chose the scope, that project is read and the bead drawn, +as *The excluded projects stay known to the run* says. Where the reader typed +the scope, the line says the bead is in that project, which was not read. +Otherwise it names the ones that may hold it: those stating the prefix, or failing any, those stating none. Where no configured project may hold it, the bead is in a project `bdi` is not configured to read. A parent no answer holds gets no such diff --git a/src/app/collection.rs b/src/app/collection.rs index 6d22290..d04699d 100644 --- a/src/app/collection.rs +++ b/src/app/collection.rs @@ -18,13 +18,13 @@ use crate::collect::agents::Agents; use crate::collect::run::FailureKind; use crate::collect::tracker::{OpenFailure, Trackers}; use crate::collect::worktree; -use crate::config::{Config, Project}; +use crate::config::{Config, Project, Scope}; use crate::model::join::{self, Listed, ProjectRows}; use crate::model::snapshot::{ self, AgentProvider, Collected, FailedProject, Filter, ProviderState, Said, Session, SessionState, Snapshot, TrackerFailure, TrackerState, Tree, }; -use crate::model::tree::{Across, Assembled, Nesting}; +use crate::model::tree::{Across, Assembled, Nesting, Unreachable}; use crate::model::types::{Bead, Pane}; use super::tracker::{open_failure, refresh_project, ProjectWork, ReadAt, Refresh, RootUnread}; @@ -156,10 +156,16 @@ pub struct Collection { /// answer can place the id a silent session was holding. What it last /// answered with can. panes_last_answered: BTreeMap>, + /// The projects the scope left out that held a drawn bead's blocker. Each + /// is read from the collection that needed it onwards. Kept here + /// rather than written into a config, so a config the reader rewrites + /// does not stop them being read. + read_on_demand: BTreeSet, } impl Collection { - /// Read what `wanted` names, and draw everything standing. + /// Read what `wanted` names, and draw everything standing — reading as + /// well any project the scope left out that holds a drawn bead's blocker. pub fn collect( &mut self, cfg: &Config, @@ -178,7 +184,55 @@ impl Collection { // only chance the agent join gets. let (panes, provider, out_of_reach) = self.every_pane(agents); - for (project, answer) in self.refresh_together(cfg, trackers, wanted, &panes, now) { + let mut reading = self.widened(cfg); + self.read( + &reading, + trackers, + &|project| wanted.names(project), + &panes, + now, + ); + loop { + match self.draw(&reading, &panes, &out_of_reach, &provider, filter, now) { + Ok(snapshot) => return snapshot, + Err(needed) => { + self.read_on_demand.extend(needed.iter().cloned()); + reading = self.widened(cfg); + self.read( + &reading, + trackers, + &|project| needed.contains(project), + &panes, + now, + ); + } + } + } + } + + /// `cfg`, with its scope taking in every project read on demand that it + /// can. + fn widened<'c>(&self, cfg: &'c Config) -> Cow<'c, Config> { + let mut widened = Cow::Borrowed(cfg); + for project in &self.read_on_demand { + if widened.scope.widens_to(project) { + widened.to_mut().scope.widen_to(project); + } + } + widened + } + + /// Refresh every project `cfg` reads that `named` names, and keep what + /// each said. + fn read( + &mut self, + cfg: &Config, + trackers: &dyn Trackers, + named: &(dyn Fn(&str) -> bool + Sync), + panes: &[Pane], + now: DateTime, + ) { + for (project, answer) in self.refresh_together(cfg, trackers, named, panes, now) { match answer { Ok(Refresh::Unchanged) => { // A skipped read is a successful read: `bdi` knows the @@ -215,11 +269,9 @@ impl Collection { } } } - - self.draw(cfg, &panes, &out_of_reach, provider, filter, now) } - /// One refresh of every project `wanted` names, made together rather + /// One refresh of every project `named` names, made together rather /// than in turn, and handed back once the last of them has answered. /// /// The trackers are independent and a read of one is a sequence of round @@ -232,14 +284,14 @@ impl Collection { &self, cfg: &'a Config, trackers: &dyn Trackers, - wanted: &Wanted, + named: &(dyn Fn(&str) -> bool + Sync), panes: &[Pane], now: DateTime, ) -> Vec<(&'a Project, Result)> { std::thread::scope(|reads| { let reading: Vec<_> = cfg .read() - .filter(|p| wanted.names(&p.name)) + .filter(|p| named(&p.name)) .map(|project| { let standing = self .read @@ -263,16 +315,17 @@ impl Collection { } /// Everything standing, in config order, however much of it this - /// collection just read. + /// collection just read. Where a project `cfg`'s scope would take in holds + /// a drawn bead's blocker, it hands back those projects instead. fn draw( &self, cfg: &Config, panes: &[Pane], out_of_reach: &BTreeSet, - agents: AgentProvider, + agents: &AgentProvider, filter: Filter, now: DateTime, - ) -> Snapshot { + ) -> Result> { let answered: Vec<(&str, &ProjectWork)> = self.that_answered(cfg).collect(); let not_read: Vec<(&str, Option<&str>)> = cfg .projects @@ -281,6 +334,10 @@ impl Collection { .filter(|(project, _)| !answered.iter().any(|(answering, _)| answering == project)) .collect(); let drawn = reaching_across(&answered, ¬_read); + let needed = held_by_unread(&drawn, &cfg.scope); + if !needed.is_empty() { + return Err(needed); + } // One resolve over every project's rows at once. A pane names its bead // by id alone, and only the whole set tells a match from a prefix @@ -360,7 +417,7 @@ impl Collection { .filter_map(|(project, work)| Some((project.to_string(), work.speaks_until?))) .collect(); - snapshot::build( + Ok(snapshot::build( Collected { trees, failed_projects, @@ -370,10 +427,10 @@ impl Collection { panes, joined, cfg, - agents, + agents.clone(), filter, now, - ) + )) } /// What has been read, in the order the config names the projects. The @@ -521,6 +578,22 @@ fn reaching_across<'a>( .collect() } +/// The projects holding a blocker some drawn bead waits on, where `scope` +/// would take them in. +fn held_by_unread(drawn: &[Drawn<'_>], scope: &Scope) -> BTreeSet { + drawn + .iter() + .filter_map(|(_, _, read)| read.as_ref().ok()) + .flat_map(|assembled| assembled.orphaned.values().flatten()) + .filter_map(|orphaned| match &orphaned.why { + Unreachable::HeldByUnread { project } if scope.widens_to(project) => { + Some(project.clone()) + } + _ => None, + }) + .collect() +} + /// A tree's beads, gathered under the project whose answer holds each. fn by_project<'a>(project: &'a str, assembled: &'a Assembled) -> Vec<(&'a str, Vec)> { let mut rows: BTreeMap<&str, Vec> = BTreeMap::new(); @@ -577,8 +650,9 @@ mod tests { use crate::model::anomaly::Anomaly; use crate::model::join::{BeadKey, Conflict}; use crate::model::snapshot::LoosePane; + use crate::model::tree::Unreachable; use crate::model::types::testing::{key, A_SESSION}; - use crate::model::types::PaneStatus; + use crate::model::types::{PaneStatus, Status}; use pretty_assertions::assert_eq; use std::collections::BTreeSet; use std::path::{Path, PathBuf}; @@ -2484,4 +2558,254 @@ path = "{}" ); assert_eq!(after.read_at["dunwich"], later); } + + // ---- a blocker in a project the run is not reading ----------------- + + const STATES_ITS_PREFIX: &str = r#"prefix = "dun""#; + const STATES_NO_PREFIX: &str = ""; + + /// dunwich, with `dunwich_states` written into its entry, and ferry — as + /// a run started in ferry's checkout reads them. + fn reading_ferry_where_dunwich(dunwich_states: &str) -> Config { + Config::from_toml(&format!( + r#" +[[projects]] +name = "dunwich" +path = "{DUNWICH}" +{dunwich_states} + +[[projects]] +name = "ferry" +path = "{FERRY}" +"# + )) + .expect("the config parses") + .scoped_to_the_project_holding(Path::new(FERRY)) + } + + fn ferry_waiting_on_dunwich() -> Fakes { + Fakes::default() + .with("dunwich", dunwich_tracker()) + .with("ferry", Fake::holding(beads(WAITING_ON_DUNWICH))) + } + + fn collected_under(cfg: &Config, trackers: &Fakes) -> Snapshot { + Collection::default().collect( + cfg, + &no_panes(), + trackers, + &Wanted::Everything, + Filter::All, + now(), + ) + } + + /// Why ferry's bead says each blocker it waits on is not drawn. + fn unreachable_from_ferry(snap: &Snapshot) -> Vec<(&str, &Unreachable)> { + node(tree_of(snap, "ferry"), "fer-2") + .orphaned_dependencies + .iter() + .map(|orphaned| (orphaned.id.as_str(), &orphaned.why)) + .collect() + } + + #[test] + fn a_blocker_a_project_the_run_is_not_reading_holds_is_read_and_drawn_as_the_bead_it_is() { + let snap = collected_under( + &reading_ferry_where_dunwich(STATES_ITS_PREFIX), + &ferry_waiting_on_dunwich(), + ); + + let blocker = node(tree_of(&snap, "ferry"), "dun-7"); + assert_eq!( + ( + blocker.project.as_str(), + blocker.title.as_str(), + &blocker.status + ), + ("dunwich", "lift the ground station", &Status::InProgress) + ); + assert_eq!(unreachable_from_ferry(&snap), vec![]); + assert_eq!(snap.projects, ["dunwich", "ferry"]); + } + + /// dunwich's bead waits on kadath's, so reading dunwich for ferry's + /// blocker is what says kadath is needed too. + #[test] + fn a_blocker_of_a_blocker_read_on_demand_is_read_on_demand_too() { + let cfg = Config::from_toml(&format!( + r#" +[[projects]] +name = "dunwich" +path = "{DUNWICH}" +prefix = "dun" + +[[projects]] +name = "ferry" +path = "{FERRY}" + +[[projects]] +name = "kadath" +path = "/srv/work/kadath" +prefix = "kad" +"# + )) + .expect("the config parses") + .scoped_to_the_project_holding(Path::new(FERRY)); + let trackers = Fakes::default() + .with( + "dunwich", + Fake::holding(beads( + r#"[ + {"id":"dun-7","title":"lift the ground station","status":"in_progress", + "dependencies":[{"depends_on_id":"kad-1","type":"blocks"}], + "priority":1,"issue_type":"epic"} + ]"#, + )), + ) + .with("ferry", Fake::holding(beads(WAITING_ON_DUNWICH))) + .with( + "kadath", + Fake::holding(beads( + r#"[ + {"id":"kad-1","title":"survey the plateau","status":"open", + "priority":2,"issue_type":"task"} + ]"#, + )), + ); + + let snap = collected_under(&cfg, &trackers); + + assert_eq!(node(tree_of(&snap, "ferry"), "kad-1").project, "kadath"); + assert_eq!(snap.projects, ["dunwich", "ferry", "kadath"]); + } + + #[test] + fn a_project_nothing_drawn_needs_is_still_not_read() { + let trackers = Fakes::default() + .with("dunwich", dunwich_tracker()) + .with("ferry", colliding_tracker()); + + let snap = collected_under(&reading_ferry_where_dunwich(STATES_ITS_PREFIX), &trackers); + + assert_eq!(asked_of(&trackers, "dunwich"), 0); + assert_eq!(snap.projects, ["ferry"]); + } + + #[test] + fn a_project_read_on_demand_is_read_again_when_a_refresh_names_it() { + let cfg = reading_ferry_where_dunwich(STATES_ITS_PREFIX); + let trackers = ferry_waiting_on_dunwich(); + let mut standing = Collection::default(); + let later = now() + chrono::Duration::seconds(30); + standing.collect( + &cfg, + &no_panes(), + &trackers, + &Wanted::Everything, + Filter::All, + now(), + ); + let asked_before = asked_of(&trackers, "dunwich"); + + let after = standing.collect( + &cfg, + &no_panes(), + &trackers, + &dunwich_alone(), + Filter::All, + later, + ); + + assert!(asked_of(&trackers, "dunwich") > asked_before); + assert_eq!(after.read_at["dunwich"], later); + } + + /// Watched from then on, as a project the run was started reading is, + /// rather than dropped the moment its beads stop being needed. + #[test] + fn a_project_read_on_demand_goes_on_being_read_once_nothing_needs_it() { + let cfg = reading_ferry_where_dunwich(STATES_ITS_PREFIX); + let mut standing = Collection::default(); + standing.collect( + &cfg, + &no_panes(), + &ferry_waiting_on_dunwich(), + &Wanted::Everything, + Filter::All, + now(), + ); + let no_longer_waiting = Fakes::default() + .with("dunwich", dunwich_tracker()) + .with("ferry", colliding_tracker()); + + let after = standing.collect( + &cfg, + &no_panes(), + &no_longer_waiting, + &Wanted::Everything, + Filter::All, + now(), + ); + + assert!(asked_of(&no_longer_waiting, "dunwich") > 0); + assert_eq!(after.projects, ["dunwich", "ferry"]); + } + + #[test] + fn a_blocker_in_a_project_stating_no_prefix_is_reported_as_it_was() { + let trackers = ferry_waiting_on_dunwich(); + + let snap = collected_under(&reading_ferry_where_dunwich(STATES_NO_PREFIX), &trackers); + + assert_eq!(asked_of(&trackers, "dunwich"), 0); + assert_eq!( + unreachable_from_ferry(&snap), + vec![( + "dun-7", + &Unreachable::NotRead { + projects: vec!["dunwich".to_string()] + } + )] + ); + } + + #[test] + fn a_blocker_no_configured_project_states_the_prefix_of_is_reported_as_it_was() { + let trackers = Fakes::default().with("dunwich", dunwich_tracker()).with( + "ferry", + Fake::holding(beads(&WAITING_ON_DUNWICH.replace("dun-7", "kad-1"))), + ); + + let snap = collected_under(&reading_ferry_where_dunwich(STATES_ITS_PREFIX), &trackers); + + assert_eq!(asked_of(&trackers, "dunwich"), 0); + assert_eq!( + unreachable_from_ferry(&snap), + vec![("kad-1", &Unreachable::Unconfigured)] + ); + } + + /// `--project` is the reader saying which projects to read, so a + /// blocker outside it is reported rather than read. + #[test] + fn a_blocker_outside_the_projects_the_reader_named_is_reported_rather_than_read() { + let cfg = reading_ferry_where_dunwich(STATES_ITS_PREFIX) + .scoped_to(&["ferry".to_string()]) + .expect("ferry is configured"); + let trackers = ferry_waiting_on_dunwich(); + + let snap = collected_under(&cfg, &trackers); + + assert_eq!(asked_of(&trackers, "dunwich"), 0); + assert_eq!( + unreachable_from_ferry(&snap), + vec![( + "dun-7", + &Unreachable::HeldByUnread { + project: "dunwich".to_string() + } + )] + ); + } } diff --git a/src/config.rs b/src/config.rs index a122335..eae1854 100644 --- a/src/config.rs +++ b/src/config.rs @@ -87,6 +87,25 @@ impl Scope { } } } + + /// Whether this scope would take `name` in were it asked to: only a + /// scope the directory chose widens, since one the reader typed says + /// what to read. + pub fn widens_to(&self, name: &str) -> bool { + matches!(self, Scope::Directory { .. }) && !self.reads(name) + } + + /// Take `name` in where [`Self::widens_to`] says it would, and say + /// whether it did. + pub fn widen_to(&mut self, name: &str) -> bool { + if !self.widens_to(name) { + return false; + } + if let Scope::Directory { widened, .. } = self { + widened.push(name.to_string()); + } + true + } } /// An unknown key is refused rather than dropped, which is serde's default. @@ -959,17 +978,14 @@ impl Config { names_of(&self.projects).join(", ") ); } - if !self.reads(project) { - match &mut self.scope { - Scope::Directory { widened, .. } => widened.push(project.to_string()), - Scope::Asked(_) | Scope::Everything => anyhow::bail!( - "{named}: bdi is not reading {project}; it is reading {}", - self.read() - .map(|p| p.name.as_str()) - .collect::>() - .join(", ") - ), - } + if !self.reads(project) && !self.scope.widen_to(project) { + anyhow::bail!( + "{named}: bdi is not reading {project}; it is reading {}", + self.read() + .map(|p| p.name.as_str()) + .collect::>() + .join(", ") + ); } Ok((project.to_string(), id.to_string())) } diff --git a/src/tui/armed.rs b/src/tui/armed.rs index b8d8741..1ae8069 100644 --- a/src/tui/armed.rs +++ b/src/tui/armed.rs @@ -11,6 +11,7 @@ use chrono::{DateTime, Utc}; use super::due::due_after; use crate::app::Wanted; +use crate::config::{Config, Scope}; /// One disarmed `Armed` per project a config names. /// @@ -20,7 +21,20 @@ use crate::app::Wanted; /// settled where `bdi` is run. The loop asks this again whenever the reader /// writes a config, so the set of projects that poll is the set the file /// names. -pub(crate) type Arming = Box Vec>; +pub(crate) type Arming = Box Vec>; + +/// Armed for each project `cfg` names and does not read, for the collection +/// that reads one on demand. +pub(super) fn armed_unread(arms: &Arming, cfg: &Config) -> Vec { + let every_project = Config { + scope: Scope::Everything, + ..cfg.clone() + }; + arms(&every_project) + .into_iter() + .filter(|armed| !cfg.reads(armed.project())) + .collect() +} /// One project's poll: how long after a read it asks to be read again, and /// when that next falls due. @@ -156,12 +170,17 @@ impl Armed { speaks_until: Option>, ) { if wanted.names(&self.project) { - self.speaks_until = speaks_until; - self.at = self.due_from(at); - self.vouched_at = Some(at); + self.was_read(at, speaks_until); } } + /// This project has been read, whatever was asked for. + pub(super) fn was_read(&mut self, at: DateTime, speaks_until: Option>) { + self.speaks_until = speaks_until; + self.at = self.due_from(at); + self.vouched_at = Some(at); + } + /// Something outside says it covers this project and nothing in it has /// moved, which says of its rows what a read that found nothing would /// have said. So the poll is pushed out from here as a read's return @@ -224,6 +243,28 @@ mod tests { Armed::polling("arkham".to_string(), Some(EVERY)) } + #[test] + fn the_projects_armed_unread_are_the_ones_the_config_names_and_the_run_does_not_read() { + let cfg = Config::from_toml( + "[[projects]]\nname = \"arkham\"\npath = \"/srv/work/arkham\"\n\n\ + [[projects]]\nname = \"ferry\"\npath = \"/srv/work/ferry\"\n", + ) + .expect("the config parses") + .scoped_to_the_project_holding(std::path::Path::new("/srv/work/arkham")); + let arms: Arming = Box::new(|cfg: &Config| { + cfg.read() + .map(|project| Armed::polling(project.name.clone(), Some(EVERY))) + .collect() + }); + + let unread: Vec = armed_unread(&arms, &cfg) + .iter() + .map(|armed| armed.project().to_string()) + .collect(); + + assert_eq!(unread, ["ferry"]); + } + /// The largest whole number of seconds — the unit `refresh_seconds` is /// read in — that still lands inside the range an instant can hold, read /// off chrono's own last instant rather than quoted from its diff --git a/src/tui/drive.rs b/src/tui/drive.rs index e8b10a9..e047693 100644 --- a/src/tui/drive.rs +++ b/src/tui/drive.rs @@ -20,7 +20,7 @@ use crate::collect::panes::Answer; use crate::model::snapshot::Snapshot; use crate::view::{Action, Motion, Notch, Typing}; -use super::armed::{Armed, Arming}; +use super::armed::{armed_unread, Armed, Arming}; use super::keys::{action, typing}; use super::reload::{Reload, Reloaded}; @@ -370,13 +370,9 @@ pub(super) fn drive( // each of them does about the screen it says below. Waited::Aged => ran_out(view, drawn_at, Utc::now()), Waited::Event(event) => { - let Some(changed) = answered( - view, - &mut outstanding, - &mut reading.polling, - &mut showing, - event, - ) else { + let Some(changed) = + answered(view, &mut outstanding, &mut reading, &mut showing, event) + else { return Ok(()); }; changed @@ -466,7 +462,7 @@ fn looked_at( let Reloaded::Fresh(written) = reloaded else { return noticed; }; - reading.now_reading(arms(written)); + reading.now_reading(arms(written), armed_unread(arms, written)); outstanding.waits_out(written.tui.unanswered_after()); if ask .send(Asked::Reloaded(Box::new(written.clone()))) @@ -513,11 +509,49 @@ fn still_armed(standing: Vec, named: Vec) -> Vec { pub(super) struct Reading { polling: Vec, accepted: Reported, + /// The projects the config names and this run is not reading, each + /// armed for the collection that reads it on demand. + unread: Vec, } impl Reading { pub(super) fn of(polling: Vec, accepted: Reported) -> Self { - Self { polling, accepted } + Self { + polling, + accepted, + unread: Vec::new(), + } + } + + pub(super) fn unread(self, unread: Vec) -> Self { + Self { unread, ..self } + } + + /// Take the projects a collection has read into what the run reads, and + /// hand back those it was not reading until now. + fn read_on_demand(&mut self, read: &[String]) -> Vec { + let (gained, unread) = std::mem::take(&mut self.unread) + .into_iter() + .partition::, _>(|project| read.iter().any(|named| named == project.project())); + self.unread = unread; + if gained.is_empty() { + return Vec::new(); + } + let names = gained + .iter() + .map(|project| project.project().to_string()) + .collect(); + self.polling.extend(gained); + self.accept_what_polls(); + names + } + + fn accept_what_polls(&self) { + self.accepted.now_watching( + self.polling + .iter() + .map(|project| project.project().to_string()), + ); } /// The projects a config the reader has written names, as what the run @@ -526,11 +560,10 @@ impl Reading { /// `still_due` is where what the file settles and what its last read /// settled are told apart, so the channel is told what came out of that /// rather than what went into it. - fn now_reading(&mut self, named: Vec) { - let polling = still_armed(std::mem::take(&mut self.polling), named); - self.accepted - .now_watching(polling.iter().map(|project| project.project().to_string())); - self.polling = polling; + fn now_reading(&mut self, named: Vec, unread: Vec) { + self.polling = still_armed(std::mem::take(&mut self.polling), named); + self.unread = unread; + self.accept_what_polls(); } } @@ -639,7 +672,7 @@ fn keeps_what_is_open(event: &Event, showing: Showing) -> bool { fn answered( view: &mut dyn View, outstanding: &mut Outstanding, - armed: &mut [Armed], + reading: &mut Reading, showing: &mut Showing, event: Event, ) -> Option { @@ -808,22 +841,33 @@ fn answered( Event::Changed(wanted) => asked_for(view, outstanding, wanted), Event::Covered(covered) => { let now = Utc::now(); - for project in armed.iter_mut().filter(|armed| armed.project() == covered) { + for project in reading + .polling + .iter_mut() + .filter(|armed| armed.project() == covered) + { project.covered(now); } false } Event::Collected(snapshot) => { let now = Utc::now(); + // A project the collection read on demand was read by it as + // much as any the read named. + let gained = reading.read_on_demand(&snapshot.projects); if let Some(read) = outstanding.came_back() { // Every project that read covered now has nothing coming, so // this is where each of them arms its next ask. The only // place: a read that never comes back arms nothing, and the // project says its tracker has stopped answering rather than // being quietly polled over. - for project in armed.iter_mut() { + for project in reading.polling.iter_mut() { let speaks_until = snapshot.speaks_until.get(project.project()).copied(); - project.came_back(&read, now, speaks_until); + if gained.iter().any(|named| named == project.project()) { + project.was_read(now, speaks_until); + } else { + project.came_back(&read, now, speaks_until); + } } } view.collected(*snapshot); @@ -2985,6 +3029,69 @@ mod tests { ); } + /// A project the run was not reading, which a collection came back + /// having read for a blocker a drawn bead waits on, asks for itself from + /// then on and is accepted on the inbound channel, as a project the run + /// was started reading is. + #[test] + fn a_project_a_collection_read_on_demand_polls_and_is_accepted_from_then_on() { + let mut view = Recorder::default(); + let (ask, asked) = mpsc::channel(); + let (send, events) = mpsc::channel(); + let reported = Reported::watching(["arkham".to_string()]); + let mut outstanding = at_once(); + outstanding.ask(Wanted::Everything, Utc::now()); + + let holding = thread::spawn(move || { + let mut reached = Vec::new(); + while let Ok(one) = asked.recv_timeout(A_MOMENT) { + let polled = matches!(one, Asked::Read(Wanted::Project(_))); + if one == Asked::Read(Wanted::Everything) { + let read_ferry_too = Snapshot { + projects: vec!["arkham".to_string(), "ferry".to_string()], + ..a_snapshot() + }; + send.send(Event::Collected(Box::new(read_ferry_too))) + .expect("the loop's end of the channel is open"); + } + reached.push(one); + if polled { + break; + } + } + drop(send); + reached + }); + + drive( + &mut view, + &events, + &ask, + outstanding, + Reading::of( + vec![Armed::polling("arkham".to_string(), None)], + reported.clone(), + ) + .unread(vec![Armed::polling("ferry".to_string(), Some(AN_INTERVAL))]), + &polling_every_interval(), + nothing_watched(), + ) + .expect("the loop runs"); + + assert_eq!( + holding.join().expect("the thread ran").last(), + Some(&Asked::Read(ferry())), + "the project read on demand asked for itself once its read came back" + ); + assert_eq!( + reported.take("ferry"), + crate::collect::changes::Answer::Watched(crate::collect::changes::Heard::Changed( + "ferry".to_string() + )), + "and the channel accepts a report for it" + ); + } + /// When the band's interval is up the loop asks it to read its pane /// again, and not before: the band is what decides whether anything is /// due, so the loop asks on every wake and the first wake is the diff --git a/src/tui/mod.rs b/src/tui/mod.rs index b2eb0b3..fa910d8 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -150,7 +150,7 @@ pub fn run( &events, &ask, outstanding, - Reading::of(armed, reported), + Reading::of(armed, reported).unread(armed::armed_unread(&arms, cfg)), &arms, reload, ) diff --git a/tests/a_blocker_in_a_project_bdi_is_not_reading_is_drawn.rs b/tests/a_blocker_in_a_project_bdi_is_not_reading_is_drawn.rs new file mode 100644 index 0000000..2a09092 --- /dev/null +++ b/tests/a_blocker_in_a_project_bdi_is_not_reading_is_drawn.rs @@ -0,0 +1,205 @@ +//! A bead named on the command line, waiting on a bead in a project the run +//! was not reading, draws that blocker as the bead it is. +//! +//! Run through the binary, because main decides which projects the run reads, +//! from the directory it was started in and the bead the command line names. + +mod terminal; + +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::time::Duration; + +use serde_json::Value; +use terminal::driver::{Driven, GIVING_UP}; +use terminal::shims::ShimmedTracker; + +/// Ferry's one bead, waiting on a bead of dunwich's. +const FERRY: &str = r#"[ + {"id":"fer-2","title":"moor the barge","status":"open","priority":2, + "issue_type":"task","dependencies":[{"depends_on_id":"dun-7","type":"blocks"}]} +]"#; + +/// Dunwich's beads: the one ferry's waits on, and a tree of its own that +/// nothing ferry holds needs. +const DUNWICH: &str = r#"[ + {"id":"dun-7","title":"lift the ground station","status":"in_progress", + "priority":1,"issue_type":"epic"}, + {"id":"dun-3","title":"survey the moor","status":"in_progress", + "priority":1,"issue_type":"task"} +]"#; + +/// The title of dunwich's own tree, which is drawn where dunwich is read and +/// nothing is focused. +const DUNWICHS_OWN_TREE: &str = "survey the moor"; + +/// A `HOME` that is ferry's directory. Its config names ferry there and +/// dunwich in a directory under it, with `dunwich_states` in dunwich's entry. +fn a_home_where_dunwich(named: &str, dunwich_states: &str) -> PathBuf { + let home = std::env::temp_dir().join(format!("bdi-{named}-{}", std::process::id())); + for directory in ["dunwich", ".config/beady-eye"] { + std::fs::create_dir_all(home.join(directory)).expect("the directory is ours to make"); + } + std::fs::write( + the_config_in(&home), + format!( + "[[projects]]\nname = \"dunwich\"\npath = \"{}\"\n{dunwich_states}\n\n\ + [[projects]]\nname = \"ferry\"\npath = \"{}\"\n", + home.join("dunwich").display(), + home.display() + ), + ) + .expect("the config is ours to write"); + home +} + +fn the_config_in(home: &Path) -> PathBuf { + home.join(".config/beady-eye/config.toml") +} + +/// The trackers of a home made above, each holding its project's beads. +fn the_trackers_in(home: &Path) -> ShimmedTracker { + let tracker = ShimmedTracker::beside(home); + tracker.holds_for("dunwich", DUNWICH); + tracker.holds_for( + home.file_name() + .and_then(|name| name.to_str()) + .expect("the home is named"), + FERRY, + ); + tracker +} + +/// The snapshot `bdi fer-2 --json` prints, started in ferry's directory. +fn ferry_named_from(home: &Path) -> Value { + let out = Command::new(env!("CARGO_BIN_EXE_bdi")) + .arg("--config") + .arg(the_config_in(home)) + .args(["fer-2", "--json"]) + .envs(the_trackers_in(home).environment()) + .current_dir(home) + .output() + .expect("bdi runs"); + assert!( + out.status.success(), + "bdi exited {}: {}", + out.status, + String::from_utf8_lossy(&out.stderr) + ); + serde_json::from_slice(&out.stdout).expect("bdi prints a snapshot") +} + +/// The node for `id` in the tree rooted at ferry's bead. +fn in_ferrys_tree<'a>(snapshot: &'a Value, id: &str) -> Option<&'a Value> { + snapshot["trees"] + .as_array() + .expect("trees is an array") + .iter() + .find(|tree| tree["root"] == "fer-2") + .expect("ferry's bead draws a tree")["nodes"] + .as_array() + .expect("nodes is an array") + .iter() + .find(|node| node["id"] == id) +} + +#[test] +fn a_blocker_whose_prefix_a_configured_project_states_is_drawn_from_that_project() { + let snapshot = ferry_named_from(&a_home_where_dunwich("read-on-demand", "prefix = \"dun\"")); + + let blocker = in_ferrys_tree(&snapshot, "dun-7").expect("the blocker is drawn"); + assert_eq!( + (&blocker["project"], &blocker["title"], &blocker["status"]), + ( + &Value::from("dunwich"), + &Value::from("lift the ground station"), + &Value::from("in_progress") + ) + ); +} + +/// The control: nothing says the blocker is dunwich's, so dunwich is not +/// read and the blocker is reported where it would have hung. +#[test] +fn a_blocker_whose_prefix_no_configured_project_states_is_reported_rather_than_read() { + let snapshot = ferry_named_from(&a_home_where_dunwich("not-read-on-demand", "")); + + assert!(in_ferrys_tree(&snapshot, "dun-7").is_none()); + assert_eq!( + in_ferrys_tree(&snapshot, "fer-2").expect("ferry's bead is drawn")["orphaned_dependencies"], + serde_json::json!([{"id": "dun-7", "reason": "not-read", "projects": ["dunwich"]}]) + ); +} + +const ROWS: u16 = 40; + +/// Wide enough for the foot to keep its notice and its keys together. +const COLS: u16 = 160; + +/// A gap this long between bytes means the frame is drawn. +const A_SILENCE: Duration = Duration::from_millis(300); + +/// `/`, ferry's bead and Enter, which leave the selection on that bead. +const FIND_FERRYS_BEAD: &[u8] = b"/fer-2\r"; + +/// Shift+F, which focuses the forest on the selected bead. +const FOCUS: &[u8] = b"F"; + +/// Naming a bead starts the forest as Shift+F on it would, and a project read +/// on demand for that bead is folded with the rest. +#[test] +fn a_project_read_on_demand_for_a_named_bead_is_folded_as_focusing_the_bead_folds_it() { + let mut by_hand = launched("focused-by-hand", &[]); + let unfocused = forest(&repaint(&mut by_hand, ROWS + 1)); + assert!( + unfocused.iter().any(|row| row.contains(DUNWICHS_OWN_TREE)), + "dunwich's own tree is not drawn, so nothing read it on demand and \ + there is nothing for focus to fold: {unfocused:#?}" + ); + + by_hand.send(FIND_FERRYS_BEAD); + by_hand.settle(A_SILENCE, GIVING_UP); + by_hand.send(FOCUS); + by_hand.settle(A_SILENCE, GIVING_UP); + let focused = forest(&repaint(&mut by_hand, ROWS)); + assert!( + !focused.iter().any(|row| row.contains(DUNWICHS_OWN_TREE)), + "focusing ferry's bead left dunwich's own tree drawn: {focused:#?}" + ); + + let mut named = launched("focused-by-name", &["fer-2"]); + assert_eq!(forest(&repaint(&mut named, ROWS + 1)), focused); +} + +/// A `bdi` started in ferry's directory, given `arguments`, with dunwich +/// stating its prefix, and its first collection drawn. +fn launched(named: &str, arguments: &[&str]) -> Driven { + let home = a_home_where_dunwich(named, "prefix = \"dun\""); + let mut environment = the_trackers_in(&home).environment(); + environment.push(terminal::a_socket_of_its_own(&home)); + let mut bdi = Driven::bdi_with_arguments(ROWS, COLS, home, arguments, &environment); + bdi.read_until(terminal::ENTER_ALTERNATE_SCREEN, GIVING_UP); + bdi.settle(A_SILENCE, GIVING_UP); + bdi +} + +/// The screen as it stands, rather than as it differs from the frame before: +/// a resize is answered by drawing every cell again. +#[track_caller] +fn repaint(bdi: &mut Driven, rows: u16) -> Vec { + let repainted = bdi.resize(rows, COLS); + bdi.answer_to(repainted, GIVING_UP) +} + +/// The forest's rows, down to the line saying what the run reads. A project +/// line is cut at the age of its read, which differs between two runs. +fn forest(screen: &[u8]) -> Vec { + terminal::rows_drawn(screen) + .into_iter() + .take_while(|row| !row.trim_start().starts_with("reading ")) + .map(|row| match row.split_once(" ✓") { + Some((project, _)) => project.to_string(), + None => row.trim_end().to_string(), + }) + .collect() +} diff --git a/tests/shims/bd b/tests/shims/bd index 754111d..f251033 100755 --- a/tests/shims/bd +++ b/tests/shims/bd @@ -13,7 +13,10 @@ # call as bdi asks it past whatever flags open it: # `list --all --limit 0 --json`, `blocked --json`, # `where --json`. A call with a file there is -# answered with it; any other is refused. +# answered with it; any other is refused. A file in +# a directory under it named for the last component +# of the tracker `-C` names answers for that +# tracker alone, ahead of one beside it. # BDI_SHIM_BD_UNANSWERED file this appends each call it had no answer for # to, so a reader can tell a capture that was not # served from one that was served wrongly. @@ -61,6 +64,19 @@ asked() { } asked=$(asked "$@") +# The last component of the directory `-C` names, or nothing where no `-C` +# opens the call. +tracker() { + while [ $# -gt 1 ]; do + case $1 in + -C) printf '%s' "${2##*/}"; return ;; + -*) shift ;; + *) return ;; + esac + done +} +tracker=$(tracker "$@") + # Written down before anything below can hold, refuse or answer the call, so # the record is of what bd was asked rather than of how asking it went. if [ -n "${BDI_SHIM_BD_CALLED:-}" ]; then @@ -103,6 +119,11 @@ case $asked in ;; esac +if [ -n "${BDI_SHIM_BD_ANSWERS:-}" ] && [ -n "$tracker" ] && + [ -f "$BDI_SHIM_BD_ANSWERS/$tracker/$asked" ]; then + exec cat "$BDI_SHIM_BD_ANSWERS/$tracker/$asked" +fi + if [ -n "${BDI_SHIM_BD_ANSWERS:-}" ] && [ -f "$BDI_SHIM_BD_ANSWERS/$asked" ]; then exec cat "$BDI_SHIM_BD_ANSWERS/$asked" fi diff --git a/tests/terminal/shims.rs b/tests/terminal/shims.rs index 6633920..97ed856 100644 --- a/tests/terminal/shims.rs +++ b/tests/terminal/shims.rs @@ -108,28 +108,46 @@ impl ShimmedTracker { /// It holds no wisps, and reports nothing ready and nothing blocked, /// until a test that needs one of those says otherwise. pub fn holds(&self, capture: &str) { + self.holds_in(&self.answers, capture); + } + + /// The same, for the tracker at a directory whose last component is + /// `tracker` alone, so that projects a run reads together can hold + /// different beads. + pub fn holds_for(&self, tracker: &str, capture: &str) { + self.holds_in(&self.answers.join(tracker), capture); + } + + fn holds_in(&self, answers: &Path, capture: &str) { let rows: Vec = serde_json::from_str(capture).expect("a capture of bd list --json"); - std::fs::create_dir_all(&self.answers).expect("the answers are ours to write"); + std::fs::create_dir_all(answers).expect("the answers are ours to write"); + let answer = |asked: &str, with: &[serde_json::Value]| { + std::fs::write( + answers.join(asked), + serde_json::to_string(with).expect("rows serialise"), + ) + .expect("the answer is ours to write"); + }; - self.answers("list --all --limit 0 --json", &rows); + answer("list --all --limit 0 --json", &rows); let unfinished: Vec = rows .iter() .filter(|row| row["status"] != "closed") .cloned() .collect(); - self.answers( + answer( &format!("list --status {UNFINISHED} --limit 0 --json"), &unfinished, ); for row in &rows { let id = row["id"].as_str().expect("a bd row names its bead"); - self.answers(&format!("show {id} --json"), std::slice::from_ref(row)); + answer(&format!("show {id} --json"), std::slice::from_ref(row)); } - self.answers("query ephemeral=true --limit 0 --json", &[]); - self.answers("query ephemeral=true --all --limit 0 --json", &[]); - self.answers("ready --limit 0 --json", &[]); - self.answers("blocked --json", &[]); + answer("query ephemeral=true --limit 0 --json", &[]); + answer("query ephemeral=true --all --limit 0 --json", &[]); + answer("ready --limit 0 --json", &[]); + answer("blocked --json", &[]); } /// Say `path` is a directory beads tracks, which is what a `bdi` given no @@ -149,10 +167,6 @@ impl ShimmedTracker { ); } - fn answers(&self, asked: &str, with: &[serde_json::Value]) { - self.answers_with(asked, &serde_json::to_string(with).expect("rows serialise")); - } - fn answers_with(&self, asked: &str, text: &str) { std::fs::write(self.answers.join(asked), text).expect("the answer is ours to write"); }