git-ents.gitmain
⌘K
foforge
commit e442c80
refactor: split WebComponent's sync loader into a Loadable supertrait

Issue only needs the shared spawn_blocking loader (issues_page keeps its own open/closed filter chrome, which the generic card has no hook for) — forcing it to also implement TITLE/empty for a card() it never calls was dead code. Loadable now carries just load(); WebComponent adds TITLE/empty/Render/Component on top for the components that do render through card().

refactor: add web::component::Loadable, implement it for Check/Member/Issue Assisted-by: Claude:claude-sonnet-5

Joseph D. Carpinelli · 1 month ago

Reviews

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

Start a review

verdict

crates/git-ents-server/src/web/component.rs @@ -4,11 +4,14 @@ //! //! Not every component's page fits this shape — Issues filters to open-only //! and shows dual open/closed counts in place of a single badge, chrome this -//! card does not have a hook for — so [`super::pages::issues_page`] reuses -//! only [`load`] and each issue's `Render` impl, keeping its own header and -//! body. Forcing that case through [`card`] would mean adding a header -//! override parameter used by exactly one component, which the component -//! plan's own trait-bloat rule rules out. +//! card does not have a hook for — so [`super::pages::issues_page`] implements +//! only [`Loadable`] and reuses [`load`], keeping its own header and body via +//! each issue's `Render` impl. Forcing that case through [`card`] would mean +//! adding a header override parameter used by exactly one component, which +//! the component plan's own trait-bloat rule rules out; splitting the sync +//! loader from the card chrome into two traits is the same rule applied the +//! other way, so Issue is not forced to implement a `TITLE`/`empty` it would +//! never use. use std::path::Path; @@ -17,29 +20,33 @@ use super::render::Render; -/// A meta-ref component whose items list and render generically: identity -/// metadata and a [`Render`] impl (both from `git_ents::component`), plus how -/// its items load and what a card shows when there are none yet. -pub(super) trait WebComponent: Component + Render + Send + Sized + 'static { - /// The card/page title. - const TITLE: &'static str; - /// The card body's empty-state message. - fn empty() -> Markup; - /// The component's items. Sync — `git_ents::*::load`/`list` shell out to - /// git and read the object database synchronously — so callers wrap it in - /// exactly one [`load`]. +/// A meta-ref component whose items load off the async runtime. Sync — +/// `git_ents::*::load`/`list` shell out to git and read the object database +/// synchronously — so callers wrap it in exactly one [`load`]. +pub(super) trait Loadable: Send + Sized + 'static { + /// The component's items. fn load(repo: &Path) -> Result<Vec<Self>, String>; } -/// Load `T`'s items off the async runtime, wrapping [`WebComponent::load`] in -/// the one `spawn_blocking` every component needs. -pub(super) async fn load<T: WebComponent>(repo: &Path) -> Result<Vec<T>, String> { +/// Load `T`'s items off the async runtime, wrapping [`Loadable::load`] in the +/// one `spawn_blocking` every component needs. +pub(super) async fn load<T: Loadable>(repo: &Path) -> Result<Vec<T>, String> { let repo = repo.to_owned(); tokio::task::spawn_blocking(move || T::load(&repo)) .await .map_err(|err| err.to_string())? } +/// A [`Loadable`] component whose items also render as a generic [`card`]: +/// identity metadata and a [`Render`] impl (both from `git_ents::component`), +/// plus a title and what the card shows when there are no items yet. +pub(super) trait WebComponent: Loadable + Component + Render { + /// The card title. + const TITLE: &'static str; + /// The card body's empty-state message. + fn empty() -> Markup; +} + /// The card chrome every list-shaped component shares: a header with /// `T::TITLE` and a count badge, then an error row, `T::empty()`, or one /// rendered row per item.
crates/git-ents-server/src/web/render.rs @@ -18,7 +18,7 @@ use git_ents::issues::Issue; use git_ents::members::Member; -use super::component::WebComponent; +use super::component::{Loadable, WebComponent}; use crate::asciidoc; /// HTML rendering for a meta-ref value. The default walks the value's [`Facet`] @@ -51,16 +51,18 @@ } } +impl Loadable for Check { + fn load(repo: &Path) -> Result<Vec<Self>, String> { + git_ents::checks::load(repo).map_err(|err| err.to_string()) + } +} + impl WebComponent for Check { const TITLE: &'static str = "Checks"; fn empty() -> Markup { html! { div.card-row.muted { "No checks configured on " code { "refs/meta/checks" } "." } } } - - fn load(repo: &Path) -> Result<Vec<Self>, String> { - git_ents::checks::load(repo).map_err(|err| err.to_string()) - } } /// Config's editable fields (description, homepage, topics) get their own @@ -109,13 +111,7 @@ } } -impl WebComponent for Issue { - const TITLE: &'static str = "Bug reports"; - - fn empty() -> Markup { - html! { div.card-row.muted { "No bug reports yet." } } - } - +impl Loadable for Issue { fn load(repo: &Path) -> Result<Vec<Self>, String> { Ok(git_ents::issues::list(repo) .map_err(|err| err.to_string())? @@ -144,6 +140,12 @@ } } +impl Loadable for Member { + fn load(repo: &Path) -> Result<Vec<Self>, String> { + git_ents::members::load_all(repo).map_err(|err| err.to_string()) + } +} + impl WebComponent for Member { const TITLE: &'static str = "Members"; @@ -152,10 +154,6 @@ div.card-row.muted { "No members — pushes are open until the first key is added." } } } - - fn load(repo: &Path) -> Result<Vec<Self>, String> { - git_ents::members::load_all(repo).map_err(|err| err.to_string()) - } } /// A run is a set of per-check outcomes; collapse them to one summary line