git-ents.gitmain
⌘K
foforge
commit 6173bfb
model, anchor, cli, web: canonical lowercase display for status, member state, and projection

Give each closed vocabulary one Display/label impl next to its type (ents_model::Status, ents_model::MemberState, ents_anchor::Projection::label) so the CLI and web UI stop hand-rolling divergent Debug-leak or duplicate match renderings, following the Verdict Display precedent. Review list/show also now abbreviates review.target() consistently with the ref-segment column it already abbreviated: the two are not redundant (the ref segment stays genesis-keyed while the target field advances on re-review, per the existing re-review test), so both stay, both abbreviated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Joseph D. Carpinelli · 28 days ago

Reviews

No reviews of this commit yet — record a verdict below.

Start a review

verdict

crates/cli/ents-web/src/render.rs @@ -26,10 +26,11 @@ /// pair per field, in declaration order. /// /// A field's value renders via its own `Display` impl when it has one -/// (plain text, no `Type::Foo(...)` wrapper -- what a `String` or a -/// `MemberId` newtype gives), and falls back to `Debug` otherwise (every -/// entity struct in `ents-model`/`ents-forge`/`ents-kiln` derives `Debug`, -/// so an enum field like [`ents_model::MemberState`] still renders its +/// (plain text, no `Type::Foo(...)` wrapper -- what a `String`, a +/// `MemberId` newtype, or an enum like [`ents_model::MemberState`] with its +/// own canonical `Display` gives), and falls back to `Debug` otherwise +/// (every entity struct in `ents-model`/`ents-forge`/`ents-kiln` derives +/// `Debug`, so a field of an enum with no `Display` still renders its /// variant name rather than an opaque placeholder). A field this crate /// cannot even walk as a struct (called on a non-struct `T`) renders as an /// empty list, not a panic -- reflection is a UI convenience, never a @@ -46,7 +47,7 @@ /// assert_eq!(rows[1].0, "key"); /// assert!(rows[1].1.contains("ssh-ed25519")); /// assert_eq!(rows[2].0, "state"); -/// assert_eq!(rows[2].1, "Active"); +/// assert_eq!(rows[2].1, "active"); /// ``` #[must_use] pub fn fields<T: Facet<'static>>(value: &T) -> Vec<FieldRow> { @@ -344,7 +345,7 @@ .iter() .find(|(name, _)| *name == "state") .expect("state field"); - assert_eq!(state, "Active"); + assert_eq!(state, "active"); assert_eq!(member.state, MemberState::Active); }
crates/cli/git-ents/src/exe.rs @@ -119,7 +119,7 @@ for (username, member) in commands::members::list(&root.refs, &root.objects)? { let _ = writeln!( out, - "{username}\t{:?}\t{:?}", + "{username}\t{}\t{:?}", member.state, member.provenance ); } @@ -146,7 +146,7 @@ } MembersAction::Check { key } => match commands::members::check(&root, key)? { Some((username, state)) => { - let _ = writeln!(out, "{username}\t{state:?}"); + let _ = writeln!(out, "{username}\t{state}"); } None => { let _ = writeln!(out, "not a member"); @@ -184,7 +184,11 @@ let (effect, status) = commands::effect::show(&root, &name, at)?; let _ = writeln!(out, "trigger: {}", effect.trigger); let _ = writeln!(out, "run: {}", effect.run); - let _ = writeln!(out, "result: {status:?}"); + let _ = writeln!( + out, + "result: {}", + status.map_or_else(|| "none".to_owned(), |status| status.to_string()) + ); } EffectAction::Add { name, @@ -204,7 +208,7 @@ } EffectAction::Log { name } => { for (oid, status) in commands::effect::log(&root, &name)? { - let _ = writeln!(out, "{oid}\t{status:?}"); + let _ = writeln!(out, "{oid}\t{status}"); } } } @@ -325,7 +329,18 @@ if let Some((anchor, projection)) = projected { let _ = writeln!(out, "path: {}", anchor.path); let target = if worktree { "worktree" } else { rev.as_str() }; - let _ = writeln!(out, "projection at {target}: {projection:?}"); + let detail = match &projection { + ents_anchor::Projection::Relocated { + path, + lines: Some(range), + } => format!(" ({path}:{}-{})", range.start, range.end), + ents_anchor::Projection::Relocated { path, lines: None } + | ents_anchor::Projection::Outdated { path } => format!(" ({path})"), + ents_anchor::Projection::Current | ents_anchor::Projection::Deleted => { + String::new() + } + }; + let _ = writeln!(out, "projection at {target}: {}{detail}", projection.label()); } let _ = writeln!(out, "body: {}", comment.body); } @@ -481,14 +496,18 @@ out, "{}\t{member}\t{}\t{}", ents_forge::abbreviate_id(&review_target), - review.target(), + ents_forge::abbreviate_id(&review.target().to_string()), review.verdict ); } } ReviewAction::Show { target, member } => { let (review, thread) = commands::review::show(&root, &target, &member)?; - let _ = writeln!(out, "target: {}", review.target()); + let _ = writeln!( + out, + "target: {}", + ents_forge::abbreviate_id(&review.target().to_string()) + ); let _ = writeln!(out, "verdict: {}", review.verdict); let _ = writeln!(out, "body: {}", review.body); for (comment_id, comment) in thread {
crates/kernel/ents-anchor/src/projection.rs @@ -55,6 +55,21 @@ Deleted, } +impl Projection { + /// The outcome's canonical lowercase keyword -- the porcelain grammar's + /// own vocabulary (`current`, `relocated`, `outdated`, `deleted`), shared + /// by every surface that names an outcome without narrating its payload. + #[must_use] + pub fn label(&self) -> &'static str { + match self { + Self::Current => "current", + Self::Relocated { .. } => "relocated", + Self::Outdated { .. } => "outdated", + Self::Deleted => "deleted", + } + } +} + /// Project `anchor` onto `target` (a revision in `repo`), degrading to /// [`project_from_context`] once `anchor`'s own commit has been garbage /// collected (`anchor.fuzzy-fallback`) — the one entry point most callers @@ -456,6 +471,16 @@ Some(LineRange { start, end }) } + #[rstest] + #[case::current(Projection::Current, "current")] + #[case::relocated(Projection::Relocated { path: "f".to_owned(), lines: None }, "relocated")] + #[case::outdated(Projection::Outdated { path: "f".to_owned() }, "outdated")] + #[case::deleted(Projection::Deleted, "deleted")] + // @relation(anchor.projection, scope=function, role=Verifies) + fn label_is_the_porcelain_keyword(#[case] projection: Projection, #[case] expected: &str) { + assert_eq!(projection.label(), expected); + } + /// One post-capture edit per taxonomy row of /// [`projection_reports_the_spec_outcomes`]. #[derive(Debug, Clone, Copy)]
crates/kernel/ents-model/src/member.rs @@ -83,6 +83,15 @@ Revoked, } +impl std::fmt::Display for MemberState { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match self { + Self::Active => "active", + Self::Revoked => "revoked", + }) + } +} + /// How a member came to be enrolled. /// /// `model.member-provenance` ties authorization for canonical refs to this @@ -237,6 +246,14 @@ assert_eq!(member.state, MemberState::Active); } + #[rstest] + #[case::active(MemberState::Active, "active")] + #[case::revoked(MemberState::Revoked, "revoked")] + // @relation(model.member-revocation, scope=function, role=Verifies) + fn member_state_displays_lowercase(#[case] state: MemberState, #[case] expected: &str) { + assert_eq!(state.to_string(), expected); + } + #[rstest] #[case::active(MemberState::Active)] #[case::revoked(MemberState::Revoked)]
crates/kernel/ents-model/src/result.rs @@ -36,6 +36,16 @@ Error, } +impl std::fmt::Display for Status { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match self { + Self::Pass => "pass", + Self::Fail => "fail", + Self::Error => "error", + }) + } +} + /// A recorded result: the outcome of running one effect against one /// commit, living at `refs/meta/results/<effect>/<short-oid>` /// (`namespace::result_ref`) or the self-run mirror. @@ -114,6 +124,15 @@ use super::*; + #[rstest] + #[case::pass(Status::Pass, "pass")] + #[case::fail(Status::Fail, "fail")] + #[case::error(Status::Error, "error")] + // @relation(model.result-taxonomy, scope=function, role=Verifies) + fn status_displays_lowercase(#[case] status: Status, #[case] expected: &str) { + assert_eq!(status.to_string(), expected); + } + #[rstest] #[case::pass(Status::Pass)] #[case::fail(Status::Fail)]
crates/cli/ents-web/src/pages/comments.rs @@ -253,7 +253,8 @@ } /// One human sentence for a projection result (`anchor.projection`) -- -/// never the enum's `Debug` form. +/// never the enum's `Debug` form; the outdated case names +/// [`Projection::label`]'s own word rather than repeating it as a literal. fn projection_label(projection: &Projection) -> String { match projection { Projection::Current => "anchored lines unchanged".to_owned(), @@ -261,7 +262,10 @@ format!("moved to {path}{}", line_label(*lines)) } Projection::Outdated { path } => { - format!("outdated \u{2014} the anchored lines in {path} have been edited") + format!( + "{} \u{2014} the anchored lines in {path} have been edited", + projection.label() + ) } Projection::Deleted => "the anchored file no longer exists".to_owned(), }
crates/cli/ents-web/src/pages/members.rs @@ -10,7 +10,7 @@ use std::sync::Arc; use axum::extract::{Path, State}; -use ents_model::{Member, MemberState, Provenance}; +use ents_model::{Member, Provenance}; use gix_object::{Find, Write}; use maud::{Markup, html}; @@ -118,7 +118,7 @@ @if let Some(key_type) = key_type { span.key-badge { (key_type) } } - span.badge { (state_label(member.state)) } + span.badge { (member.state) } span.badge { (provenance_label(member.provenance)) } } div.member-key { @@ -161,14 +161,6 @@ format!("{head}\u{2026}{tail}") } -/// [`MemberState`] as its badge text. -fn state_label(state: MemberState) -> &'static str { - match state { - MemberState::Active => "active", - MemberState::Revoked => "revoked", - } -} - /// [`Provenance`] as its badge text. fn provenance_label(provenance: Provenance) -> &'static str { match provenance {
crates/cli/ents-web/src/pages/mod.rs @@ -662,14 +662,11 @@ html! { span class={ "status status-" (class) } { (label) } } } -/// The `.status-<word>` chip for a closed pass/fail/error [`ents_model::Status`]. +/// The `.status-<word>` chip for a closed pass/fail/error [`ents_model::Status`], +/// its word taken straight from the type's own `Display`. pub(crate) fn status_chip(status: ents_model::Status) -> Markup { - let word = match status { - ents_model::Status::Pass => "pass", - ents_model::Status::Fail => "fail", - ents_model::Status::Error => "error", - }; - status_chip_labeled(word, word) + let word = status.to_string(); + status_chip_labeled(&word, &word) } /// A `.verdict-<word>` chip for a review's own [`ents_forge::review::Verdict`].
crates/cli/git-ents/src/commands/comment.rs @@ -228,12 +228,13 @@ anchor: &ents_anchor::Anchor, ) -> (String, String) { use ents_anchor::Projection; - match projection { - Projection::Current => ("current".to_owned(), location(&anchor.path, anchor.lines)), - Projection::Relocated { path, lines } => ("relocated".to_owned(), location(path, *lines)), - Projection::Outdated { path } => ("outdated".to_owned(), location(path, None)), - Projection::Deleted => ("deleted".to_owned(), "-".to_owned()), - } + let location = match projection { + Projection::Current => location(&anchor.path, anchor.lines), + Projection::Relocated { path, lines } => location(path, *lines), + Projection::Outdated { path } => location(path, None), + Projection::Deleted => "-".to_owned(), + }; + (projection.label().to_owned(), location) } fn location(path: &str, lines: Option<ents_anchor::LineRange>) -> String {