From f01151db0d261b3eef363d3f9dcb5d2e5daadc6e Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Fri, 12 Jan 2024 08:33:59 -0600 Subject: [PATCH] Move formatting of explanations into the report formatter (#19) --- examples/unsat_root_message_no_version.rs | 101 ++++++- src/report.rs | 309 ++++++++++++---------- 2 files changed, 263 insertions(+), 147 deletions(-) diff --git a/examples/unsat_root_message_no_version.rs b/examples/unsat_root_message_no_version.rs index cdb1154b..294ff569 100644 --- a/examples/unsat_root_message_no_version.rs +++ b/examples/unsat_root_message_no_version.rs @@ -2,7 +2,7 @@ use pubgrub::error::PubGrubError; use pubgrub::range::Range; -use pubgrub::report::Reporter; +use pubgrub::report::{Derived, Reporter}; use pubgrub::solver::{resolve, OfflineDependencyProvider}; use pubgrub::version::SemanticVersion; @@ -108,6 +108,105 @@ impl ReportFormatter> for CustomReportFormatter } } } + + /// Simplest case, we just combine two external incompatibilities. + fn explain_both_external( + &self, + external1: &External>, + external2: &External>, + current_terms: &Map>>, + ) -> String { + // TODO: order should be chosen to make it more logical. + format!( + "Because {} and {}, {}.", + self.format_external(external1), + self.format_external(external2), + self.format_terms(current_terms) + ) + } + + /// Both causes have already been explained so we use their refs. + fn explain_both_ref( + &self, + ref_id1: usize, + derived1: &Derived>, + ref_id2: usize, + derived2: &Derived>, + current_terms: &Map>>, + ) -> String { + // TODO: order should be chosen to make it more logical. + format!( + "Because {} ({}) and {} ({}), {}.", + self.format_terms(&derived1.terms), + ref_id1, + self.format_terms(&derived2.terms), + ref_id2, + self.format_terms(current_terms) + ) + } + + /// One cause is derived (already explained so one-line), + /// the other is a one-line external cause, + /// and finally we conclude with the current incompatibility. + fn explain_ref_and_external( + &self, + ref_id: usize, + derived: &Derived>, + external: &External>, + current_terms: &Map>>, + ) -> String { + // TODO: order should be chosen to make it more logical. + format!( + "Because {} ({}) and {}, {}.", + self.format_terms(&derived.terms), + ref_id, + self.format_external(external), + self.format_terms(current_terms) + ) + } + + /// Add an external cause to the chain of explanations. + fn and_explain_external( + &self, + external: &External>, + current_terms: &Map>>, + ) -> String { + format!( + "And because {}, {}.", + self.format_external(external), + self.format_terms(current_terms) + ) + } + + /// Add an already explained incompat to the chain of explanations. + fn and_explain_ref( + &self, + ref_id: usize, + derived: &Derived>, + current_terms: &Map>>, + ) -> String { + format!( + "And because {} ({}), {}.", + self.format_terms(&derived.terms), + ref_id, + self.format_terms(current_terms) + ) + } + + /// Add an already explained incompat to the chain of explanations. + fn and_explain_prior_and_external( + &self, + prior_external: &External>, + external: &External>, + current_terms: &Map>>, + ) -> String { + format!( + "And because {} and {}, {}.", + self.format_external(prior_external), + self.format_external(external), + self.format_terms(current_terms) + ) + } } fn main() { diff --git a/src/report.rs b/src/report.rs index bf6cbc28..54261c17 100644 --- a/src/report.rs +++ b/src/report.rs @@ -249,6 +249,58 @@ pub trait ReportFormatter { /// Format terms of an incompatibility. fn format_terms(&self, terms: &Map>) -> Self::Output; + + /// Simplest case, we just combine two external incompatibilities. + fn explain_both_external( + &self, + external1: &External, + external2: &External, + current_terms: &Map>, + ) -> Self::Output; + + /// Both causes have already been explained so we use their refs. + fn explain_both_ref( + &self, + ref_id1: usize, + derived1: &Derived, + ref_id2: usize, + derived2: &Derived, + current_terms: &Map>, + ) -> Self::Output; + + /// One cause is derived (already explained so one-line), + /// the other is a one-line external cause, + /// and finally we conclude with the current incompatibility. + fn explain_ref_and_external( + &self, + ref_id: usize, + derived: &Derived, + external: &External, + current_terms: &Map>, + ) -> Self::Output; + + /// Add an external cause to the chain of explanations. + fn and_explain_external( + &self, + external: &External, + current_terms: &Map>, + ) -> Self::Output; + + /// Add an already explained incompat to the chain of explanations. + fn and_explain_ref( + &self, + ref_id: usize, + derived: &Derived, + current_terms: &Map>, + ) -> Self::Output; + + /// Add an already explained incompat to the chain of explanations. + fn and_explain_prior_and_external( + &self, + prior_external: &External, + external: &External, + current_terms: &Map>, + ) -> Self::Output; } /// Default formatter for the default reporter. @@ -281,6 +333,105 @@ impl ReportFormatter for DefaultStringReportF } } } + + /// Simplest case, we just combine two external incompatibilities. + fn explain_both_external( + &self, + external1: &External, + external2: &External, + current_terms: &Map>, + ) -> String { + // TODO: order should be chosen to make it more logical. + format!( + "Because {} and {}, {}.", + self.format_external(external1), + self.format_external(external2), + self.format_terms(current_terms) + ) + } + + /// Both causes have already been explained so we use their refs. + fn explain_both_ref( + &self, + ref_id1: usize, + derived1: &Derived, + ref_id2: usize, + derived2: &Derived, + current_terms: &Map>, + ) -> String { + // TODO: order should be chosen to make it more logical. + format!( + "Because {} ({}) and {} ({}), {}.", + self.format_terms(&derived1.terms), + ref_id1, + self.format_terms(&derived2.terms), + ref_id2, + self.format_terms(current_terms) + ) + } + + /// One cause is derived (already explained so one-line), + /// the other is a one-line external cause, + /// and finally we conclude with the current incompatibility. + fn explain_ref_and_external( + &self, + ref_id: usize, + derived: &Derived, + external: &External, + current_terms: &Map>, + ) -> String { + // TODO: order should be chosen to make it more logical. + format!( + "Because {} ({}) and {}, {}.", + self.format_terms(&derived.terms), + ref_id, + self.format_external(external), + self.format_terms(current_terms) + ) + } + + /// Add an external cause to the chain of explanations. + fn and_explain_external( + &self, + external: &External, + current_terms: &Map>, + ) -> String { + format!( + "And because {}, {}.", + self.format_external(external), + self.format_terms(current_terms) + ) + } + + /// Add an already explained incompat to the chain of explanations. + fn and_explain_ref( + &self, + ref_id: usize, + derived: &Derived, + current_terms: &Map>, + ) -> String { + format!( + "And because {} ({}), {}.", + self.format_terms(&derived.terms), + ref_id, + self.format_terms(current_terms) + ) + } + + /// Add an already explained incompat to the chain of explanations. + fn and_explain_prior_and_external( + &self, + prior_external: &External, + external: &External, + current_terms: &Map>, + ) -> String { + format!( + "And because {} and {}, {}.", + self.format_external(prior_external), + self.format_external(external), + self.format_terms(current_terms) + ) + } } /// Default reporter able to generate an explanation as a [String]. @@ -330,11 +481,10 @@ impl DefaultStringReporter { match (current.cause1.deref(), current.cause2.deref()) { (DerivationTree::External(external1), DerivationTree::External(external2)) => { // Simplest case, we just combine two external incompatibilities. - self.lines.push(Self::explain_both_external( + self.lines.push(formatter.explain_both_external( external1, external2, ¤t.terms, - formatter, )); } (DerivationTree::Derived(derived), DerivationTree::External(external)) => { @@ -354,34 +504,25 @@ impl DefaultStringReporter { ) { // If both causes already have been referenced (shared_id), // the explanation simply uses those references. - (Some(ref1), Some(ref2)) => self.lines.push(Self::explain_both_ref( + (Some(ref1), Some(ref2)) => self.lines.push(formatter.explain_both_ref( ref1, derived1, ref2, derived2, ¤t.terms, - formatter, )), // Otherwise, if one only has a line number reference, // we recursively call the one without reference and then // add the one with reference to conclude. (Some(ref1), None) => { self.build_recursive(derived2, formatter); - self.lines.push(Self::and_explain_ref( - ref1, - derived1, - ¤t.terms, - formatter, - )); + self.lines + .push(formatter.and_explain_ref(ref1, derived1, ¤t.terms)); } (None, Some(ref2)) => { self.build_recursive(derived1, formatter); - self.lines.push(Self::and_explain_ref( - ref2, - derived2, - ¤t.terms, - formatter, - )); + self.lines + .push(formatter.and_explain_ref(ref2, derived2, ¤t.terms)); } // Finally, if no line reference exists yet, // we call recursively the first one and then, @@ -400,11 +541,10 @@ impl DefaultStringReporter { let ref1 = self.ref_count; self.lines.push("".into()); self.build_recursive(derived2, formatter); - self.lines.push(Self::and_explain_ref( + self.lines.push(formatter.and_explain_ref( ref1, derived1, ¤t.terms, - formatter, )); } } @@ -425,12 +565,11 @@ impl DefaultStringReporter { formatter: &F, ) { match self.line_ref_of(derived.shared_id) { - Some(ref_id) => self.lines.push(Self::explain_ref_and_external( + Some(ref_id) => self.lines.push(formatter.explain_ref_and_external( ref_id, derived, external, current_terms, - formatter, )), None => self.report_recurse_one_each(derived, external, current_terms, formatter), } @@ -453,152 +592,30 @@ impl DefaultStringReporter { // we can chain the external explanations. (DerivationTree::Derived(prior_derived), DerivationTree::External(prior_external)) => { self.build_recursive(prior_derived, formatter); - self.lines.push(Self::and_explain_prior_and_external( + self.lines.push(formatter.and_explain_prior_and_external( prior_external, external, current_terms, - formatter, )); } // If the derived cause has itself one external prior cause, // we can chain the external explanations. (DerivationTree::External(prior_external), DerivationTree::Derived(prior_derived)) => { self.build_recursive(prior_derived, formatter); - self.lines.push(Self::and_explain_prior_and_external( + self.lines.push(formatter.and_explain_prior_and_external( prior_external, external, current_terms, - formatter, )); } _ => { self.build_recursive(derived, formatter); - self.lines.push(Self::and_explain_external( - external, - current_terms, - formatter, - )); + self.lines + .push(formatter.and_explain_external(external, current_terms)); } } } - // String explanations ##################################################### - - /// Simplest case, we just combine two external incompatibilities. - fn explain_both_external< - P: Package, - VS: VersionSet, - F: ReportFormatter, - >( - external1: &External, - external2: &External, - current_terms: &Map>, - formatter: &F, - ) -> String { - // TODO: order should be chosen to make it more logical. - format!( - "Because {} and {}, {}.", - formatter.format_external(external1), - formatter.format_external(external2), - formatter.format_terms(current_terms) - ) - } - - /// Both causes have already been explained so we use their refs. - fn explain_both_ref>( - ref_id1: usize, - derived1: &Derived, - ref_id2: usize, - derived2: &Derived, - current_terms: &Map>, - formatter: &F, - ) -> String { - // TODO: order should be chosen to make it more logical. - format!( - "Because {} ({}) and {} ({}), {}.", - formatter.format_terms(&derived1.terms), - ref_id1, - formatter.format_terms(&derived2.terms), - ref_id2, - formatter.format_terms(current_terms) - ) - } - - /// One cause is derived (already explained so one-line), - /// the other is a one-line external cause, - /// and finally we conclude with the current incompatibility. - fn explain_ref_and_external< - P: Package, - VS: VersionSet, - F: ReportFormatter, - >( - ref_id: usize, - derived: &Derived, - external: &External, - current_terms: &Map>, - formatter: &F, - ) -> String { - // TODO: order should be chosen to make it more logical. - format!( - "Because {} ({}) and {}, {}.", - formatter.format_terms(&derived.terms), - ref_id, - formatter.format_external(external), - formatter.format_terms(current_terms) - ) - } - - /// Add an external cause to the chain of explanations. - fn and_explain_external< - P: Package, - VS: VersionSet, - F: ReportFormatter, - >( - external: &External, - current_terms: &Map>, - formatter: &F, - ) -> String { - format!( - "And because {}, {}.", - formatter.format_external(external), - formatter.format_terms(current_terms) - ) - } - - /// Add an already explained incompat to the chain of explanations. - fn and_explain_ref>( - ref_id: usize, - derived: &Derived, - current_terms: &Map>, - formatter: &F, - ) -> String { - format!( - "And because {} ({}), {}.", - formatter.format_terms(&derived.terms), - ref_id, - formatter.format_terms(current_terms) - ) - } - - /// Add an already explained incompat to the chain of explanations. - fn and_explain_prior_and_external< - P: Package, - VS: VersionSet, - F: ReportFormatter, - >( - prior_external: &External, - external: &External, - current_terms: &Map>, - formatter: &F, - ) -> String { - format!( - "And because {} and {}, {}.", - formatter.format_external(prior_external), - formatter.format_external(external), - formatter.format_terms(current_terms) - ) - } - // Helper functions ######################################################## fn add_line_ref(&mut self) {