From 05dbf1bdfe100ffb841057c59e4eb2f4c15ca5dd Mon Sep 17 00:00:00 2001 From: Schneems Date: Fri, 26 Jan 2024 17:22:12 -0600 Subject: [PATCH] Replace announce struct with additional state It was identified that calling `announce().warning("").end_announce` is "weird" which I agree with. It exists due to a desire to tame newlines when toggling between multiple warnings/errors and "normal" output, more below. The prior implementation was shaped by the limitations of boxed traits. With that out of the way, this implementation swaps `struct Announce` for `BuildpackOutput>` where T is the "return type". Imagine the states going something like this: ``` Section -> Announce
-> Section or Section -> Announce
-> Announce
-> Section or Started -> Announce -> Started or Started -> Announce -> Announce -> Started ``` The `Announce` state is entered any time a `warning()` or `important()` is called. It is exited when a function from SectionMarker or StartedMarker is called that will output to the UI. This works, because those functions always return a `BuildpackOutput
` or `BuildpackOutput` and effectively act as an exit of the Announce state. The downside to this approach is that any methods that do not consume, must be implemented separately, like `mut_step()`. --- .../src/output/buildpack_output.rs | 259 +++++++----------- .../src/output/inline_output.rs | 8 +- 2 files changed, 109 insertions(+), 158 deletions(-) diff --git a/libherokubuildpack/src/output/buildpack_output.rs b/libherokubuildpack/src/output/buildpack_output.rs index 987dcb1b..1a98a58c 100644 --- a/libherokubuildpack/src/output/buildpack_output.rs +++ b/libherokubuildpack/src/output/buildpack_output.rs @@ -15,7 +15,6 @@ //! output.finish(); //! ``` //! -//! To output inside of a layer see [`inline_output`]. use crate::output::style; use std::fmt::Debug; use std::io::Write; @@ -28,7 +27,7 @@ use std::time::Instant; pub struct BuildpackOutput { pub(crate) io: W, pub(crate) data: BuildData, - pub(crate) _state: T, + pub(crate) state: T, } /// A bag of data passed throughout the lifecycle of a [`BuildpackOutput`] @@ -58,6 +57,89 @@ pub(crate) mod state { #[derive(Debug)] pub struct Section; + + #[derive(Debug)] + pub struct Announce(pub T); +} + +#[doc(hidden)] +pub trait StartedMarker {} +impl StartedMarker for state::Started {} +impl StartedMarker for state::Announce where S: StartedMarker + IntoAnnounceMarker {} + +#[doc(hidden)] +pub trait SectionMarker {} +impl SectionMarker for state::Section {} +impl SectionMarker for state::Announce where S: SectionMarker + IntoAnnounceMarker {} + +#[doc(hidden)] +pub trait IntoAnnounceMarker {} +impl IntoAnnounceMarker for state::Section {} +impl IntoAnnounceMarker for state::Started {} + +impl BuildpackOutput, W> +where + W: Write + Send + Sync + Debug + 'static, +{ + #[must_use] + pub fn warning(mut self, s: &str) -> BuildpackOutput, W> { + writeln_now(&mut self.io, style::warning(s.trim())); + writeln_now(&mut self.io, ""); + + self + } + + #[must_use] + pub fn important(mut self, s: &str) -> BuildpackOutput, W> { + writeln_now(&mut self.io, style::important(s.trim())); + writeln_now(&mut self.io, ""); + + self + } + + pub fn error(mut self, s: &str) { + writeln_now(&mut self.io, style::error(s.trim())); + writeln_now(&mut self.io, ""); + } +} + +impl BuildpackOutput +where + S: IntoAnnounceMarker, + W: Write + Send + Sync + Debug + 'static, +{ + #[must_use] + pub fn warning(mut self, s: &str) -> BuildpackOutput, W> { + writeln_now(&mut self.io, ""); + + let announce = BuildpackOutput { + io: self.io, + data: self.data, + state: state::Announce(self.state), + }; + announce.warning(s) + } + + #[must_use] + pub fn important(mut self, s: &str) -> BuildpackOutput, W> { + writeln_now(&mut self.io, ""); + + let announce = BuildpackOutput { + io: self.io, + data: self.data, + state: state::Announce(self.state), + }; + announce.important(s) + } + + pub fn error(self, s: &str) { + let announce = BuildpackOutput { + io: self.io, + data: self.data, + state: state::Announce(self.state), + }; + announce.error(s); + } } impl BuildpackOutput @@ -67,7 +149,7 @@ where pub fn new(io: W) -> Self { Self { io, - _state: state::NotStarted, + state: state::NotStarted, data: BuildData::default(), } } @@ -85,13 +167,14 @@ where BuildpackOutput { io: self.io, data: self.data, - _state: state::Started, + state: state::Started, } } } -impl BuildpackOutput +impl BuildpackOutput where + S: StartedMarker, W: Write + Send + Sync + Debug + 'static, { pub fn section(mut self, s: &str) -> BuildpackOutput { @@ -100,7 +183,7 @@ where BuildpackOutput { io: self.io, data: self.data, - _state: state::Section, + state: state::Section, } } @@ -111,15 +194,6 @@ where writeln_now(&mut self.io, style::section(format!("Done {details}"))); self.io } - - pub fn announce(self) -> Announce { - Announce { - io: self.io, - data: self.data, - _state: state::Started, - leader: Some("\n".to_string()), - } - } } impl BuildpackOutput @@ -129,16 +203,26 @@ where pub fn mut_step(&mut self, s: &str) { writeln_now(&mut self.io, style::step(s)); } +} +impl BuildpackOutput +where + S: SectionMarker, + W: Write + Send + Sync + Debug + 'static, +{ #[must_use] pub fn step(mut self, s: &str) -> BuildpackOutput { - self.mut_step(s); + writeln_now(&mut self.io, style::step(s)); - self + BuildpackOutput { + io: self.io, + data: self.data, + state: state::Section, + } } pub fn step_timed_stream(mut self, s: &str) -> Stream { - self.mut_step(s); + writeln_now(&mut self.io, style::step(s)); let started = Instant::now(); let arc_io = Arc::new(Mutex::new(self.io)); @@ -156,112 +240,7 @@ where BuildpackOutput { io: self.io, data: self.data, - _state: state::Started, - } - } - - pub fn announce(self) -> Announce { - Announce { - io: self.io, - data: self.data, - _state: state::Section, - leader: Some("\n".to_string()), - } - } -} - -// Store internal state, print leading character exactly once on warning or important -#[derive(Debug)] -pub struct Announce -where - W: Write + Send + Sync + Debug + 'static, -{ - io: W, - data: BuildData, - _state: T, - leader: Option, -} - -impl Announce -where - T: Debug, - W: Write + Send + Sync + Debug + 'static, -{ - fn shared_warning(&mut self, s: &str) { - if let Some(leader) = self.leader.take() { - write_now(&mut self.io, leader); - } - - writeln_now(&mut self.io, style::warning(s.trim())); - writeln_now(&mut self.io, ""); - } - - fn shared_important(&mut self, s: &str) { - if let Some(leader) = self.leader.take() { - write_now(&mut self.io, leader); - } - writeln_now(&mut self.io, style::important(s.trim())); - writeln_now(&mut self.io, ""); - } - - pub fn error(mut self, s: &str) { - if let Some(leader) = self.leader.take() { - write_now(&mut self.io, leader); - } - writeln_now(&mut self.io, style::error(s.trim())); - writeln_now(&mut self.io, ""); - } -} - -impl Announce -where - W: Write + Send + Sync + Debug + 'static, -{ - #[must_use] - pub fn warning(mut self, s: &str) -> Announce { - self.shared_warning(s); - - self - } - - #[must_use] - pub fn important(mut self, s: &str) -> Announce { - self.shared_important(s); - - self - } - - pub fn end_announce(self) -> BuildpackOutput { - BuildpackOutput { - io: self.io, - data: self.data, - _state: state::Section, - } - } -} - -impl Announce -where - W: Write + Send + Sync + Debug + 'static, -{ - #[must_use] - pub fn warning(mut self, s: &str) -> Announce { - self.shared_warning(s); - self - } - - #[must_use] - pub fn important(mut self, s: &str) -> Announce { - self.shared_important(s); - self - } - - #[must_use] - pub fn end_announce(self) -> BuildpackOutput { - BuildpackOutput { - io: self.io, - data: self.data, - _state: state::Started, + state: state::Started, } } } @@ -344,7 +323,7 @@ where let mut section = BuildpackOutput { io, data: self.data, - _state: state::Section, + state: state::Section, }; section.mut_step(&format!( @@ -444,9 +423,7 @@ mod test { .start("RCT") .section("Guest thoughs") .step("The scenery here is wonderful") - .announce() .warning("It's too crowded here\nI'm tired") - .end_announce() .step("The jumping fountains are great") .step("The music is nice here") .end_section() @@ -476,13 +453,11 @@ mod test { let output = BuildpackOutput::new(writer) .start("RCT") .section("Guest thoughts") - .step("The scenery here is wonderful") - .announce(); + .step("The scenery here is wonderful"); let io = output .warning("It's too crowded here") .warning("I'm tired") - .end_announce() .step("The jumping fountains are great") .step("The music is nice here") .end_section() @@ -506,28 +481,4 @@ mod test { assert_eq!(expected, strip_control_codes(String::from_utf8_lossy(&io))); } - - #[test] - fn announce_and_exit_makes_no_whitespace() { - let writer = Vec::new(); - let io = BuildpackOutput::new(writer) - .start("Quick and simple") - .section("Start") - .step("Step") - .announce() // <== Here - .end_announce() // <== Here - .end_section() - .finish(); - - let expected = formatdoc! {" - - # Quick and simple - - - Start - - Step - - Done (finished in < 0.1s) - "}; - - assert_eq!(expected, strip_control_codes(String::from_utf8_lossy(&io))); - } } diff --git a/libherokubuildpack/src/output/inline_output.rs b/libherokubuildpack/src/output/inline_output.rs index b2f12e67..67234558 100644 --- a/libherokubuildpack/src/output/inline_output.rs +++ b/libherokubuildpack/src/output/inline_output.rs @@ -60,17 +60,17 @@ pub fn step_stream(s: impl AsRef, f: impl FnOnce(&mut Stream) -> /// Print an error block to the output pub fn error(s: impl AsRef) { - build_buildpack_output().announce().error(s.as_ref()); + build_buildpack_output().error(s.as_ref()); } /// Print an warning block to the output pub fn warning(s: impl AsRef) { - let _ = build_buildpack_output().announce().warning(s.as_ref()); + let _ = build_buildpack_output().warning(s.as_ref()); } /// Print an important block to the output pub fn important(s: impl AsRef) { - let _ = build_buildpack_output().announce().important(s.as_ref()); + let _ = build_buildpack_output().important(s.as_ref()); } fn build_buildpack_output() -> BuildpackOutput { @@ -79,6 +79,6 @@ fn build_buildpack_output() -> BuildpackOutput { // Be careful not to do anything that might access this state // as it's ephemeral data (i.e. not passed in from the start of the build) data: BuildData::default(), - _state: state::Section, + state: state::Section, } }