Render the escapes in an error, and drop the manifest field nobody read

Two loose ends from the per-component work.

The card's error is the tail of what the build actually printed, and a
compiler marks its own errors in colour -- so it arrived as `[1;31merror`,
with punctuation welded onto the one line somebody is trying to read. It now
goes through the same `ansiAnnotated` the log dialog uses: the colour is
drawn, and the sequences with no meaning on a phone are consumed rather than
printed, so a cursor movement cannot arrive looking like corruption either.
One composable covers it because every failure already went through
OutputText. Selection copies AnnotatedString.text, which is the message with
every escape already gone, so what lands on the clipboard is what was on
screen rather than what was on the wire.

And `/manifest` carried a `build` object that nothing on the phone ever
parsed. It was going to be how a running build survived leaving the app;
that is not built, so it is gone rather than left looking finished.
`RunningBuild` went with it -- it was split out of `BuildStatus` for that
one reader and had become an indirection with one user, so `status()` fills
one flat struct again. `/status` is byte-identical either way, since the
split was flattened on the wire; checked against the running server rather
than assumed.

Looked at on the emulator against a failing build: bold red `error`, blue
`-->`, no escape text anywhere, and long-pressing it still raises the
handles and the Copy toolbar.
This commit is contained in:
iris committed 2026-09-01 03:48:23 -04:00
1 parent 3685ab107d
commit aa8e2b97a5
4 files changed
+44 -70

No files matched your search

+9 -1
View File
@@ -441,7 +441,7 @@ mutable at runtime from the phone.
answer, and `claim` writes that entry synchronously under the same lock answer, and `claim` writes that entry synchronously under the same lock
the route answers from -- so nothing can read a component the request the route answers from -- so nothing can read a component the request
just claimed as idle, which the phone would take for "the build is just claimed as idle, which the phone would take for "the build is
over". `RunningBuild::building` stays, but it means "anything at all is over". `BuildStatus::building` stays, but it means "anything at all is
happening here" and is only for the controls that act on the whole happening here" and is only for the controls that act on the whole
checkout; anything about one component reads that component's `step`. checkout; anything about one component reads that component's `step`.
The app mirrors the split exactly: `ProjectState` for the pull and the The app mirrors the split exactly: `ProjectState` for the pull and the
@@ -473,6 +473,14 @@ mutable at runtime from the phone.
card that starts a selection on long-press fights the gestures it card that starts a selection on long-press fights the gestures it
already has. Iris asked for exactly that line on 2026-09-01: "not the already has. Iris asked for exactly that line on 2026-09-01: "not the
'failed' but the command output for build errors and stuff". 'failed' but the command output for build errors and stuff".
`OutputText` also renders the ANSI escapes rather than printing them,
through the same `ansiAnnotated` the log dialog uses -- a compiler marks
its own errors in colour and the tail of a failed build is that output
verbatim, so raw it arrived as `[1;31merror` with punctuation welded
onto the one line somebody was trying to read. Selection copies
`AnnotatedString.text`, which is the message with every escape already
gone, so what lands on the clipboard is what was on screen rather than
what was on the wire.
- **A finished component shows nothing, and its button goes back to - **A finished component shows nothing, and its button goes back to
normal.** Iris's call, 2026-09-01: "you shouldn't see the time it took normal.** Iris's call, 2026-09-01: "you shouldn't see the time it took
once it finishes, it should just go back to its normal enabled button once it finishes, it should just go back to its normal enabled button
@@ -10,6 +10,7 @@ import androidx.compose.material3.MaterialTheme
import androidx.compose.material3.Text import androidx.compose.material3.Text
import androidx.compose.material3.darkColorScheme import androidx.compose.material3.darkColorScheme
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.runtime.remember
import androidx.compose.ui.Modifier import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.Color
import androidx.compose.ui.text.TextStyle import androidx.compose.ui.text.TextStyle
@@ -206,7 +207,23 @@ fun OutputText(
color: Color = MaterialTheme.colorScheme.error, color: Color = MaterialTheme.colorScheme.error,
style: TextStyle = LocalTextStyle.current, style: TextStyle = LocalTextStyle.current,
) { ) {
SelectionContainer { Text(text, style = style, color = color) } // The same rendering the log dialog gets, and for the same reason: a
// compiler marks its own errors in colour, and the tail of a failed
// build is that output verbatim. Printed raw it read as `[1;31merror`
// -- which is not merely ugly, it is the one line of the message a
// person is trying to read with punctuation welded onto it. The
// sequences with no meaning on a phone are consumed rather than
// printed (see `ansiAnnotated`), so a cursor movement cannot arrive
// looking like corruption either.
//
// [color] stays the colour of everything the escapes did not claim,
// which is what keeps a message with no escapes in it -- git's stderr,
// this app's own fallbacks -- looking exactly as it did.
val rendered = remember(text, color) { ansiAnnotated(text, color) }
// Selection copies `AnnotatedString.text`, which is the message with
// every escape already gone -- so what lands on the clipboard is what
// was on screen rather than what was on the wire.
SelectionContainer { Text(rendered, style = style) }
} }
/** /**
+17 -50
View File
@@ -209,32 +209,20 @@ pub struct BuildState {
inner: Mutex<Inner>, inner: Mutex<Inner>,
} }
/// What `/status` answers: whether anything needs building, and what is
/// being done about it.
///
/// One struct rather than a nested one. It was split so a card on
/// `/manifest` could carry the run half without paying for `stale`, which
/// walks every component's directory -- but nothing on the phone ever read
/// that field, so the split was an indirection with one user and the
/// second half of it has gone.
#[derive(Serialize)] #[derive(Serialize)]
#[serde(rename_all = "camelCase")] #[serde(rename_all = "camelCase")]
pub struct BuildStatus { pub struct BuildStatus {
/// The expensive one: answering it walks every component's directory,
/// which is why this is on `/status` and not on the manifest path.
pub stale: bool, pub stale: bool,
/// Flattened, so `/status` answers the one flat object it always has
/// while a card can carry [`RunningBuild`] on its own. They are split
/// because `stale` is the expensive half: it walks every component's
/// directory, and `describe` is on the manifest path.
#[serde(flatten)]
pub run: RunningBuild,
}
/// What a build is doing, with nothing in it that has to be measured off
/// disk to answer.
///
/// This is the whole of what the phone needs to pick a build back up. It
/// is reported on the card as well as from `/status`, because the app's
/// record of a run lives only in the composition: leaving the app tears
/// down the polling loop and the card state with it, and without this the
/// list it comes back to cannot say that a build is still going. The
/// server never lost anything -- the run owns its own `Arc<BuildState>`
/// and outlives every request -- so the fix is for the manifest to say so
/// rather than for anything here to be re-attached to.
#[derive(Serialize, Clone)]
#[serde(rename_all = "camelCase")]
pub struct RunningBuild {
/// Anything at all is happening for this project -- a pull, or any /// Anything at all is happening for this project -- a pull, or any
/// component being built. Deliberately the *project's* question, for /// component being built. Deliberately the *project's* question, for
/// the controls that act on the whole checkout; a caller asking about /// the controls that act on the whole checkout; a caller asking about
@@ -1204,32 +1192,12 @@ impl BuildState {
.any(|run| run.name == component && run.error.is_some()) .any(|run| run.name == component && run.error.is_some())
} }
/// What this project is doing, or `None` when nothing is.
///
/// Deliberately not [`Self::status`], which also answers `stale` and
/// so walks every component's directory. This is reached from
/// `describe`, which builds every card on `/manifest` -- fetched on
/// every open, resume and Refresh -- so it is a lock and some clones
/// and nothing else.
pub fn running(&self) -> Option<RunningBuild> {
let inner = self.inner.lock().unwrap();
inner.anything_running().then(|| Self::snapshot(&inner))
}
pub fn status(&self) -> BuildStatus { pub fn status(&self) -> BuildStatus {
// Before the lock, not inside it -- see `is_stale`. // Before the lock, not inside it -- see `is_stale`.
let stale = self.is_stale(None); let stale = self.is_stale(None);
let inner = self.inner.lock().unwrap(); let inner = self.inner.lock().unwrap();
BuildStatus { BuildStatus {
stale, stale,
run: Self::snapshot(&inner),
}
}
/// The one place the run is read out of the lock, so `/status` and a
/// card cannot come to describe the same build differently.
fn snapshot(inner: &Inner) -> RunningBuild {
RunningBuild {
building: inner.anything_running(), building: inner.anything_running(),
error: inner.error.clone(), error: inner.error.clone(),
unrelated_histories: inner.unrelated_histories, unrelated_histories: inner.unrelated_histories,
@@ -1441,7 +1409,7 @@ mod tests {
move || crate::config::project_config(&project).matches_accepted(&accepted, None), move || crate::config::project_config(&project).matches_accepted(&accepted, None),
Arc::new(|_: &str, _: String| {}), Arc::new(|_: &str, _: String| {}),
); );
while state.status().run.building { while state.status().building {
tokio::time::sleep(std::time::Duration::from_millis(10)).await; tokio::time::sleep(std::time::Duration::from_millis(10)).await;
} }
@@ -1490,7 +1458,7 @@ mod tests {
state.build_now(Some("app"), Arc::new(|_: &str, _: String| {})); state.build_now(Some("app"), Arc::new(|_: &str, _: String| {}));
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10);
while state.status().run.building { while state.status().building {
assert!( assert!(
std::time::Instant::now() < deadline, std::time::Instant::now() < deadline,
"build did not finish in time" "build did not finish in time"
@@ -1510,7 +1478,6 @@ mod tests {
let status = state.status(); let status = state.status();
assert_eq!( assert_eq!(
status status
.run
.components .components
.iter() .iter()
.map(|c| c.name.as_str()) .map(|c| c.name.as_str())
@@ -1571,7 +1538,7 @@ mod tests {
std::time::Instant::now() < deadline, std::time::Instant::now() < deadline,
"quick never finished, so it was waiting on slow", "quick never finished, so it was waiting on slow",
); );
let status = state.status().run; let status = state.status();
let quick = status.components.iter().find(|c| c.name == "quick"); let quick = status.components.iter().find(|c| c.name == "quick");
if quick.is_some_and(|quick| quick.step.is_none()) { if quick.is_some_and(|quick| quick.step.is_none()) {
assert!( assert!(
@@ -1591,7 +1558,7 @@ mod tests {
"slow is still going, which is the point", "slow is still going, which is the point",
); );
while state.status().run.building { while state.status().building {
assert!( assert!(
std::time::Instant::now() < deadline, std::time::Instant::now() < deadline,
"slow did not finish in time" "slow did not finish in time"
@@ -1634,7 +1601,7 @@ mod tests {
state.build_now(None, Arc::new(|_: &str, _: String| {})); state.build_now(None, Arc::new(|_: &str, _: String| {}));
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10);
while state.status().run.building { while state.status().building {
assert!( assert!(
std::time::Instant::now() < deadline, std::time::Instant::now() < deadline,
"build did not finish in time" "build did not finish in time"
@@ -1642,7 +1609,7 @@ mod tests {
tokio::time::sleep(std::time::Duration::from_millis(10)).await; tokio::time::sleep(std::time::Duration::from_millis(10)).await;
} }
let status = state.status().run; let status = state.status();
assert!( assert!(
status.error.is_none(), status.error.is_none(),
"the project's error slot is for pulls, not for a component's build", "the project's error slot is for pulls, not for a component's build",
@@ -1723,7 +1690,7 @@ mod tests {
state.trigger_if_needed(Some("b"), Arc::new(|_: &str, _: String| {})); state.trigger_if_needed(Some("b"), Arc::new(|_: &str, _: String| {}));
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10);
while state.status().run.building { while state.status().building {
assert!( assert!(
std::time::Instant::now() < deadline, std::time::Instant::now() < deadline,
"build did not finish in time" "build did not finish in time"
-18
View File
@@ -493,23 +493,6 @@ struct ManifestApp {
/// one flag for the list, so a card says whether *it* is the one still /// one flag for the list, so a card says whether *it* is the one still
/// being worked out. /// being worked out.
check_pending: bool, check_pending: bool,
/// The build running for this project right now, if one is, in the
/// same shape `/status` answers.
///
/// Here so that a build survives leaving the app. The run itself never
/// stops -- it owns its own `Arc<BuildState>` and outlives the request
/// that started it -- but the phone's record of it lives only in the
/// composition, so backgrounding tears down the polling loop and the
/// card state together. Without this the list it comes back to cannot
/// say a build is still going, and the card reads as one that was
/// killed: it offers Update again, and pressing it does nothing,
/// because there is already a run in this project's one build slot.
///
/// Read with [`crate::build_state::BuildState::running`] rather than
/// `status`, which also answers `stale` and walks every component's
/// directory to do it -- this is on the manifest path.
#[serde(skip_serializing_if = "Option::is_none")]
build: Option<crate::build_state::RunningBuild>,
/// What this project produces, in the order it is built. One entry is /// What this project produces, in the order it is built. One entry is
/// the ordinary case and the card shows it inline; more than one is /// the ordinary case and the card shows it inline; more than one is
/// what the phone draws as a nested list. /// what the phone draws as a nested list.
@@ -673,7 +656,6 @@ async fn describe(state: &Arc<AppState>, entry: &AppEntry) -> Result<ManifestApp
.build .build
.as_ref() .as_ref()
.is_some_and(|build| build.has_command()), .is_some_and(|build| build.has_command()),
build: entry.build.as_ref().and_then(|build| build.running()),
git_ipv4: entry.git_ipv4, git_ipv4: entry.git_ipv4,
built_in: entry.built_in, built_in: entry.built_in,
pending_declaration: pending pending_declaration: pending