Skip to content

Commit

Permalink
wip: proper unicode characters in error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
@aochagavia
aochagavia committed Feb 2, 2024
1 parent f4d9f72 commit 0a56c5e
Show file tree
Hide file tree
Showing 16 changed files with 300 additions and 133 deletions.
261 changes: 217 additions & 44 deletions src/problem.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,114 @@ impl ProblemGraph {
}
}

#[derive(Copy, Clone, PartialEq, Eq)]
enum ChildOrder {
HasRemainingSiblings,
Last,
}

struct Indenter {
levels: Vec<ChildOrder>,
top_level_indent: bool,
}

impl Indenter {
fn new(top_level_indent: bool) -> Self {
Self {
levels: Vec::new(),
top_level_indent,
}
}

fn is_at_top_level(&self) -> bool {
self.levels.len() == 1
}

fn push_level(&self) -> Self {
self.push_level_with_order(ChildOrder::HasRemainingSiblings)
}

fn push_level_with_order(&self, order: ChildOrder) -> Self {
let mut levels = self.levels.clone();
levels.push(order);
Self {
levels,
top_level_indent: self.top_level_indent,
}
}

fn set_last(&mut self) {
*self.levels.last_mut().unwrap() = ChildOrder::Last;
}

fn get_indent(&self) -> String {
assert!(!self.levels.is_empty());

let mut s = String::new();

let deepest_level = self.levels.len() - 1;

for (level, &order) in self.levels.iter().enumerate() {
if level == 0 && !self.top_level_indent {
// Skip
continue;
}

let is_at_deepest_level = level == deepest_level;

let tree_prefix = match (is_at_deepest_level, order) {
(true, ChildOrder::HasRemainingSiblings) => "├─",
(true, ChildOrder::Last) => "└─",
(false, ChildOrder::HasRemainingSiblings) => "│ ",
(false, ChildOrder::Last) => " ",
};

// TODO: are these the right characters? Alternatives: https://en.wikipedia.org/wiki/Box-drawing_character or look at mamba

s.push_str(tree_prefix);
s.push(' ');
}

s
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_indenter_without_top_level_indent() {
let indenter = Indenter::new(false);

let indenter = indenter.push_level_with_order(ChildOrder::Last);
assert_eq!(indenter.get_indent(), "");

let indenter = indenter.push_level_with_order(ChildOrder::Last);
assert_eq!(indenter.get_indent(), "└─ ");
}

#[test]
fn test_indenter_with_multiple_siblings() {
let indenter = Indenter::new(true);

let indenter = indenter.push_level_with_order(ChildOrder::Last);
assert_eq!(indenter.get_indent(), "└─ ");

let indenter = indenter.push_level_with_order(ChildOrder::HasRemainingSiblings);
assert_eq!(indenter.get_indent(), " ├─ ");

let indenter = indenter.push_level_with_order(ChildOrder::Last);
assert_eq!(indenter.get_indent(), " │ └─ ");

let indenter = indenter.push_level_with_order(ChildOrder::Last);
assert_eq!(indenter.get_indent(), " │ └─ ");

let indenter = indenter.push_level_with_order(ChildOrder::HasRemainingSiblings);
assert_eq!(indenter.get_indent(), " │ ├─ ");
}
}

/// A struct implementing [`fmt::Display`] that generates a user-friendly representation of a
/// problem graph
pub struct DisplayUnsat<
Expand Down Expand Up @@ -554,19 +662,6 @@ impl<
}
}

fn get_indent(depth: usize, top_level_indent: bool) -> String {
let depth_correction = if depth > 0 && !top_level_indent { 1 } else { 0 };

let mut indent = " ".repeat((depth - depth_correction) * 4);

let display_tree_char = depth != 0 || top_level_indent;
if display_tree_char {
indent.push_str("|-- ");
}

indent
}

fn fmt_graph(
&self,
f: &mut Formatter<'_>,
Expand Down Expand Up @@ -609,6 +704,7 @@ impl<
let mut reported: HashSet<SolvableId> = HashSet::new();

// Note: we are only interested in requires edges here
let indenter = Indenter::new(top_level_indent);
let mut stack = top_level_edges
.iter()
.filter(|e| e.weight().try_requires().is_some())
Expand All @@ -623,10 +719,17 @@ impl<
.iter()

Check warning on line 719 in src/problem.rs

View workflow job for this annotation

GitHub Actions / Format and Lint 

Diff in /home/runner/work/resolvo/resolvo/src/problem.rs
.any(|&edge| installable_nodes.contains(&graph.edge_endpoints(edge).unwrap().1))
})
.map(|(version_set_id, edges)| (DisplayOp::Requirement(version_set_id, edges), 0))
.map(|(version_set_id, edges)| (DisplayOp::Requirement(version_set_id, edges), indenter.push_level()))
.collect::<Vec<_>>();
while let Some((node, depth)) = stack.pop() {
let indent = Self::get_indent(depth, top_level_indent);

if !stack.is_empty() {
// Mark the first element of the stack as not having any remaining siblings
stack[0].1.set_last();
}

while let Some((node, indenter)) = stack.pop() {
let top_level = indenter.is_at_top_level();
let indent = indenter.get_indent();

match node {
DisplayOp::Requirement(version_set_id, edges) => {
Expand All @@ -652,7 +755,7 @@ impl<
edges.len() == 1 && graph[target_nx] == ProblemNode::UnresolvedDependency;
if missing {
// No candidates for requirement
if depth == 0 {
if top_level {
writeln!(f, "{indent}No candidates were found for {name} {req}.")?;
} else {
writeln!(
Expand All @@ -662,7 +765,7 @@ impl<
}
} else if installable {
// Package can be installed (only mentioned for top-level requirements)
if depth == 0 {
if top_level {
writeln!(
f,
"{indent}{name} {req} can be installed with any of the following options:"
Expand All @@ -671,33 +774,85 @@ impl<
writeln!(f, "{indent}{name} {req}, which can be installed with any of the following options:")?;
}

stack.extend(
edges
.iter()
.filter(|&&e| {
installable_nodes.contains(&graph.edge_endpoints(e).unwrap().1)
})
.map(|&e| {
(
DisplayOp::Candidate(graph.edge_endpoints(e).unwrap().1),
depth + 1,
)
}),
);
let mut children: Vec<_> = edges
.iter()
.filter(|&&e| {
installable_nodes.contains(&graph.edge_endpoints(e).unwrap().1)
})
.map(|&e| {
(
DisplayOp::Candidate(graph.edge_endpoints(e).unwrap().1),
indenter.push_level(),
)
})
.collect();

// TODO: this is an utterly ugly hack that should be burnt to ashes
let mut deduplicated_children = Vec::new();

Check warning on line 791 in src/problem.rs

View workflow job for this annotation

GitHub Actions / Format and Lint 

Diff in /home/runner/work/resolvo/resolvo/src/problem.rs
let mut merged_and_seen = HashSet::new();
for child in children {
let (DisplayOp::Candidate(child_node), _) = child else { unreachable!() };
let solvable_id = graph[child_node].solvable_id();
let merged = self.merged_candidates.get(&solvable_id);

// Skip merged stuff that we have already seen
if merged_and_seen.contains(&solvable_id) {
continue;
}

if let Some(merged) = merged {
merged_and_seen.extend(merged.ids.iter().copied())
}

deduplicated_children.push(child);
}

if !deduplicated_children.is_empty() {
deduplicated_children[0].1.set_last();
}

stack.extend(deduplicated_children);
} else {
// Package cannot be installed (the conflicting requirement is further down the tree)
if depth == 0 {
if top_level {
writeln!(f, "{indent}{name} {req} cannot be installed because there are no viable options:")?;
} else {
writeln!(f, "{indent}{name} {req}, which cannot be installed because there are no viable options:")?;

Check warning on line 820 in src/problem.rs

View workflow job for this annotation

GitHub Actions / Format and Lint 

Diff in /home/runner/work/resolvo/resolvo/src/problem.rs
}

stack.extend(edges.iter().map(|&e| {
let children: Vec<_> = edges.iter()
.map(|&e| {
(
DisplayOp::Candidate(graph.edge_endpoints(e).unwrap().1),
depth + 1,
indenter.push_level(),
)
}));
}).collect();

// TODO: this is an utterly ugly hack that should be burnt to ashes
let mut deduplicated_children = Vec::new();
let mut merged_and_seen = HashSet::new();

Check warning on line 833 in src/problem.rs

View workflow job for this annotation

GitHub Actions / Format and Lint 

Diff in /home/runner/work/resolvo/resolvo/src/problem.rs
for child in children {
let (DisplayOp::Candidate(child_node), _) = child else { unreachable!() };
let solvable_id = graph[child_node].solvable_id();
let merged = self.merged_candidates.get(&solvable_id);

// Skip merged stuff that we have already seen
if merged_and_seen.contains(&solvable_id) {
continue;
}

if let Some(merged) = merged {
merged_and_seen.extend(merged.ids.iter().copied())
}

deduplicated_children.push(child);
}

if !deduplicated_children.is_empty() {
deduplicated_children[0].1.set_last();
}

stack.extend(deduplicated_children);
}
}
DisplayOp::Candidate(candidate) => {
Expand Down Expand Up @@ -798,35 +953,41 @@ impl<
} else if already_installed {
writeln!(f, "{indent}{name} {version}, which conflicts with the versions reported above.")?;
} else if constrains_conflict {
let version_sets = graph
let mut version_sets = graph
.edges(candidate)
.flat_map(|e| match e.weight() {
ProblemEdge::Conflict(ConflictCause::Constrains(
version_set_id,
)) => Some(version_set_id),
_ => None,
})
.dedup();
.dedup()
.peekable();

writeln!(f, "{indent}{name} {version} would constrain",)?;

let indent = Self::get_indent(depth + 1, top_level_indent);
for &version_set_id in version_sets {
let mut indenter = indenter.push_level();
while let Some(&version_set_id) = version_sets.next() {
let version_set =
self.solver_cache.pool().resolve_version_set(version_set_id);
let name = self
.solver_cache
.pool()
.resolve_version_set_package_name(version_set_id);
let name = self.solver_cache.pool().resolve_package_name(name);

if version_sets.peek().is_none() {
indenter.set_last();
}
let indent = indenter.get_indent();
writeln!(
f,
"{indent}{name} {version_set} , which conflicts with any installable versions previously reported",
)?;
}
} else {
writeln!(f, "{indent}{name} {version} would require",)?;
let requirements = graph
let mut requirements = graph
.edges(candidate)
.group_by(|e| e.weight().requires())
.into_iter()
Expand All @@ -841,8 +1002,13 @@ impl<
})

Check warning on line 1002 in src/problem.rs

View workflow job for this annotation

GitHub Actions / Format and Lint 

Diff in /home/runner/work/resolvo/resolvo/src/problem.rs
})
.map(|(version_set_id, edges)| {
(DisplayOp::Requirement(version_set_id, edges), depth + 1)
});
(DisplayOp::Requirement(version_set_id, edges), indenter.push_level())
})
.collect::<Vec<_>>();

if !requirements.is_empty() {
requirements[0].1.set_last();
}

stack.extend(requirements);
}
Expand Down Expand Up @@ -877,8 +1043,15 @@ impl<
self.fmt_graph(f, &top_level_conflicts, true)?;

// Conflicts caused by locked dependencies
let indent = Self::get_indent(0, true);
for e in self.graph.graph.edges(self.graph.root_node) {
let mut edges = self.graph.graph.edges(self.graph.root_node).peekable();
let indenter = Indenter::new(true);
while let Some(e) = edges.next() {
let indenter = indenter.push_level_with_order(match edges.peek() {
Some(_) => ChildOrder::HasRemainingSiblings,
None => ChildOrder::Last,
});
let indent = indenter.get_indent();

let conflict = match e.weight() {
ProblemEdge::Requires(_) => continue,
ProblemEdge::Conflict(conflict) => conflict,
Expand Down
14 changes: 7 additions & 7 deletions tests/snapshots/solver__excluded.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ source: tests/solver.rs
expression: "solve_snapshot(provider, &[\"a\"])"
---
The following packages are incompatible
|-- a * cannot be installed because there are no viable options:
|-- a 2 would require
|-- b *, which cannot be installed because there are no viable options:
|-- b 1 is excluded because it is externally excluded
|-- a 1 would require
|-- c *, which cannot be installed because there are no viable options:
|-- c 1 is excluded because it is externally excluded
└─ a * cannot be installed because there are no viable options:
├─ a 2 would require
│ └─ b *, which cannot be installed because there are no viable options:
│ └─ b 1 is excluded because it is externally excluded
└─ a 1 would require
└─ c *, which cannot be installed because there are no viable options:
└─ c 1 is excluded because it is externally excluded

4 changes: 2 additions & 2 deletions tests/snapshots/solver__merge_excluded.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@ source: tests/solver.rs
expression: "solve_snapshot(provider, &[\"a\"])"
---
The following packages are incompatible
|-- a * cannot be installed because there are no viable options:
|-- a >=1, <=2 is excluded because it is externally excluded
└─ a * cannot be installed because there are no viable options:
└─ a >=1, <=2 is excluded because it is externally excluded

Loading

0 comments on commit 0a56c5e

Please sign in to comment.