diff --git a/crates/dropshot-api-manager/src/compatibility/display.rs b/crates/dropshot-api-manager/src/compatibility/display.rs index 2eade6a..d5be8e7 100644 --- a/crates/dropshot-api-manager/src/compatibility/display.rs +++ b/crates/dropshot-api-manager/src/compatibility/display.rs @@ -6,9 +6,12 @@ //! module turns one of those issues into the labeled, tree-drawn output the //! user sees on `cargo openapi check`. -use super::types::{ - ApiCompatIssue, CompatRenderStatus, DocumentBasePath, DocumentPath, - PathTree, PathTreeKey, SubpathChange, +use super::{ + types::{ + ApiCompatIssue, CompatRenderStatus, DocumentBasePath, DocumentPath, + PathTree, PathTreeKey, SubpathChange, + }, + wrap::{Indent, Line, write_wrapped}, }; use crate::output::Styles; use drift::ChangeClass; @@ -33,12 +36,28 @@ pub(crate) struct ApiCompatIssueDisplay<'a> { pub(super) issue: &'a ApiCompatIssue, pub(super) styles: &'a Styles, pub(super) status: CompatRenderStatus, + /// Total terminal width available, minus any external indent + /// applied by the caller. `None` disables wrapping. + pub(super) wrap_width: Option, +} + +impl<'a> ApiCompatIssueDisplay<'a> { + /// Constrain rendering to wrap at `width` visible columns. + pub(crate) fn with_wrap_width(mut self, width: usize) -> Self { + self.wrap_width = Some(width); + self + } } impl fmt::Display for ApiCompatIssueDisplay<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let groups = collect_endpoint_groups(&self.issue.tree); let label_width = self.block_label_width(&groups); + // Continuation indent for wrapped rows and the leading prefix + // for tree-drawn lines: same length as `: ` + // so columns align across the block. + let row_indent_string = " ".repeat(label_width + 2); + let row_indent = Indent::spaces(&row_indent_string); // Newlines separate sections rather than terminate them, so the // rendered block always ends mid-line regardless of whether a @@ -49,12 +68,18 @@ impl fmt::Display for ApiCompatIssueDisplay<'_> { if i > 0 { writeln!(f)?; } - self.write_change_header(f, change, label_width, marker)?; + self.write_change_header( + f, + change, + label_width, + row_indent, + marker, + )?; marker = None; } if !groups.is_empty() { writeln!(f)?; - self.write_used_by_section(f, &groups, label_width)?; + self.write_used_by_section(f, &groups, label_width, row_indent)?; } Ok(()) } @@ -83,7 +108,7 @@ impl ChangeAnchor { } } -impl ApiCompatIssueDisplay<'_> { +impl<'a> ApiCompatIssueDisplay<'a> { /// Compute the label column width for this block. /// /// The width is the longest label that will appear, across: @@ -102,13 +127,33 @@ impl ApiCompatIssueDisplay<'_> { max } + /// Emit one rendered `Line` of content immediately after a label, + /// honoring [`Self::wrap_width`]. + /// + /// The caller is expected to have already written the right-aligned + /// label and `: ` separator (see [`write_label`]), positioning the + /// formatter at the column where `row_indent` ends. Continuation + /// lines, if any, align to the same column. + fn write_row_content( + &self, + f: &mut fmt::Formatter<'_>, + row_indent: Indent<'_>, + content: &Line<'_>, + ) -> fmt::Result { + match self.wrap_width { + Some(width) => write_wrapped(f, content, width, row_indent), + None => content.write_inline(f), + } + } + /// Render one change's severity and `at:` lines (without a trailing newline /// after `at:`). fn write_change_header( &self, f: &mut fmt::Formatter<'_>, - change: &SubpathChange, + change: &'a SubpathChange, label_width: usize, + row_indent: Indent<'_>, anchor: Option, ) -> fmt::Result { let styles = self.styles; @@ -123,18 +168,22 @@ impl ApiCompatIssueDisplay<'_> { class_style(change.class, styles), label_width, )?; - f.write_str(&change.message)?; + let mut severity_line = Line::new(); + severity_line.push_plain(change.message.as_str()); if let Some(anchor) = anchor { let text = match anchor { ChangeAnchor::Define(n) => format!("[#{n}]"), ChangeAnchor::Reference(n) => format!("(see #{n})"), }; - write!(f, " {}", text.style(styles.warning))?; + severity_line.push_plain(" ").push(text, styles.warning); } + self.write_row_content(f, row_indent, &severity_line)?; writeln!(f)?; write_label(f, "at", styles.dimmed, label_width)?; - self.write_at_content(f, change) + let mut at_line = Line::new(); + self.append_at_content(&mut at_line, change); + self.write_row_content(f, row_indent, &at_line) } /// Render `used by:` lines and their intermediate trees, without a leading @@ -142,8 +191,9 @@ impl ApiCompatIssueDisplay<'_> { fn write_used_by_section( &self, f: &mut fmt::Formatter<'_>, - groups: &[EndpointGroup<'_>], + groups: &[EndpointGroup<'a>], label_width: usize, + row_indent: Indent<'_>, ) -> fmt::Result { let styles = self.styles; // This map is used to elide shared subtrees with `(*)` back-references, @@ -157,12 +207,11 @@ impl ApiCompatIssueDisplay<'_> { writeln!(f)?; } write_label(f, "used by", styles.dimmed, label_width)?; - write_humanized_base(f, group.endpoint_base, styles)?; + let mut head = Line::new(); + append_humanized_base(&mut head, group.endpoint_base, styles); + self.write_row_content(f, row_indent, &head)?; if !group.intermediates.children.is_empty() { - let mut prefix = String::new(); - for _ in 0..(label_width + 2) { - prefix.push(' '); - } + let mut prefix = row_indent.string.to_string(); self.write_intermediate_tree( f, &group.intermediates, @@ -186,7 +235,14 @@ impl ApiCompatIssueDisplay<'_> { /// rendering. A repeated key is rendered with a trailing `(*)` and its /// subtree is suppressed — the reader can find the full chain at the /// first occurrence. - fn write_intermediate_tree<'a>( + /// + /// We don't try and wrap lines within the tree. That's because handling + /// such lines is quite complex: a line within the tree can be prefixed by + /// any number of `│ ` , and applying a wrap would require passing in the + /// right prefix. Endpoint paths inside the tree don't really happen in + /// practice, so we just let these lines overflow. Something worth + /// revisiting in the future, though! + fn write_intermediate_tree( &self, f: &mut fmt::Formatter<'_>, tree: &'a IntermediateTree<'a>, @@ -214,13 +270,15 @@ impl ApiCompatIssueDisplay<'_> { // Type name stays bolded as the row's anchor; field path // renders at default weight so the tree stays quieter than // the `at:` line above. - write_humanized_at_pair( - f, + let mut row = Line::new(); + append_humanized_at_pair( + &mut row, &key.base, &key.subpath, styles, Style::default(), - )?; + ); + row.write_inline(f)?; // If this exact key was already expanded earlier in the // block, mark `(*)` and skip the subtree. @@ -237,7 +295,7 @@ impl ApiCompatIssueDisplay<'_> { Ok(()) } - /// Write the content of the `at:` line for one [`SubpathChange`]. + /// Build the content of the `at:` line for one [`SubpathChange`]. /// /// * If the blessed and generated locations match exactly, render once. /// * For pure additions (old subpath is root, base unchanged) or @@ -253,11 +311,11 @@ impl ApiCompatIssueDisplay<'_> { /// container being a noisy artifact of drift's path stack. /// * Otherwise (rename, type change with rename, etc.) render both /// forms separated by ` -> ` so the rename is explicit. - fn write_at_content( + fn append_at_content( &self, - f: &mut fmt::Formatter<'_>, - change: &SubpathChange, - ) -> fmt::Result { + line: &mut Line<'a>, + change: &'a SubpathChange, + ) { let issue = self.issue; let styles = self.styles; let same_base = issue.blessed_base == issue.generated_base; @@ -269,31 +327,34 @@ impl ApiCompatIssueDisplay<'_> { // a quieter field style; see `write_intermediate_tree`. let field_style = styles.bold; if same_base && same_subpath { - return write_humanized_at_pair( - f, + append_humanized_at_pair( + line, &issue.blessed_base, &change.old_subpath, styles, field_style, ); + return; } if same_base && change.old_subpath.is_root() { - return write_humanized_at_pair( - f, + append_humanized_at_pair( + line, &issue.generated_base, &change.new_subpath, styles, field_style, ); + return; } if same_base && change.new_subpath.is_root() { - return write_humanized_at_pair( - f, + append_humanized_at_pair( + line, &issue.blessed_base, &change.old_subpath, styles, field_style, ); + return; } // Pair on `(blessed_is_paths_root, generated_is_paths_root)` so the @@ -304,8 +365,8 @@ impl ApiCompatIssueDisplay<'_> { ) { // Endpoint added: drop the `.paths` side and render only the // new endpoint. - (true, false) => write_humanized_at_pair( - f, + (true, false) => append_humanized_at_pair( + line, &issue.generated_base, &change.new_subpath, styles, @@ -313,8 +374,8 @@ impl ApiCompatIssueDisplay<'_> { ), // Endpoint removed: drop the `.paths` side and render only the old // endpoint. - (false, true) => write_humanized_at_pair( - f, + (false, true) => append_humanized_at_pair( + line, &issue.blessed_base, &change.old_subpath, styles, @@ -324,21 +385,21 @@ impl ApiCompatIssueDisplay<'_> { // ordinarily happen, but handle it in case it does anyway. In both // cases, render both sides with ` -> `. (true, true) | (false, false) => { - write_humanized_at_pair( - f, + append_humanized_at_pair( + line, &issue.blessed_base, &change.old_subpath, styles, field_style, - )?; - f.write_str(" -> ")?; - write_humanized_at_pair( - f, + ); + line.push_plain(" -> "); + append_humanized_at_pair( + line, &issue.generated_base, &change.new_subpath, styles, field_style, - ) + ); } } } @@ -465,104 +526,107 @@ fn walk_and_collect<'a>( ancestors.pop(); } -/// Write a base and subpath pair in the form used by `at:` and the tree-drawn -/// continuation lines. +/// Append a (`base`, `subpath`) pair to `line` in the form used by `at:` +/// and the tree-drawn continuation lines. /// -/// `field_style` is forwarded to [`write_humanized_subpath`] for components +/// `field_style` is forwarded to [`append_humanized_subpath`] for components /// and controls how the subpath portion is styled. -fn write_humanized_at_pair( - f: &mut fmt::Formatter<'_>, - base: &DocumentBasePath, - subpath: &DocumentPath, +fn append_humanized_at_pair<'a>( + line: &mut Line<'a>, + base: &'a DocumentBasePath, + subpath: &'a DocumentPath, styles: &Styles, field_style: Style, -) -> fmt::Result { - write_humanized_base(f, base, styles)?; +) { + append_humanized_base(line, base, styles); if subpath.is_root() { - return Ok(()); + return; } match base { DocumentBasePath::Component(_) => { - write_humanized_subpath(f, subpath, field_style) + append_humanized_subpath(line, subpath, field_style); } DocumentBasePath::Endpoint { .. } => { - // We call `write_jq_path` and not `write_humanized_subpath` + // We call `append_jq_path` and not `append_humanized_subpath` // here. This is because segments like `properties` are not a // structural part of endpoints (only of components). - f.write_str(" (")?; - write_jq_path(f, subpath, Style::default())?; - f.write_str(")") + line.push_plain(" ("); + append_jq_path(line, subpath, Style::default()); + line.push_plain(")"); } DocumentBasePath::PathsRoot | DocumentBasePath::Other(_) => { - write_jq_path(f, subpath, Style::default()) + append_jq_path(line, subpath, Style::default()); } } } -/// Write `base` in humanized form by dispatching on its variant: +/// Append `base` to `line` in humanized form by dispatching on its variant: /// /// * For [`DocumentBasePath::Component`], the bolded schema name. /// * For [`DocumentBasePath::Endpoint`], a green-bold `` followed by /// ``. If an `operation_id` is provided, it is appended in /// parentheses. /// * For [`DocumentBasePath::PathsRoot`], the literal `.paths`. In practice -/// this is dropped in [`Self::write_at_content`] before reaching here, so -/// this is purely defensive. +/// this is dropped in [`ApiCompatIssueDisplay::append_at_content`] before +/// reaching here, so this is purely defensive. /// * For [`DocumentBasePath::Other`], falls back to full jq notation. -fn write_humanized_base( - f: &mut fmt::Formatter<'_>, - base: &DocumentBasePath, +fn append_humanized_base<'a>( + line: &mut Line<'a>, + base: &'a DocumentBasePath, styles: &Styles, -) -> fmt::Result { +) { match base { DocumentBasePath::Component(path) => { - write!(f, "{}", component_name(path).style(styles.bold)) + line.push(component_name(path), styles.bold); } DocumentBasePath::Endpoint { name, operation_id } => { - write_endpoint_method_route(f, name, styles)?; - write_operation_id(f, operation_id.as_deref(), styles) + append_endpoint_method_route(line, name, styles); + append_operation_id(line, operation_id.as_deref(), styles); } DocumentBasePath::PathsRoot => { - write!(f, "{}", ".paths".style(styles.bold)) + line.push(".paths", styles.bold); + } + DocumentBasePath::Other(path) => { + append_jq_path(line, path, styles.bold); } - DocumentBasePath::Other(path) => write_jq_path(f, path, styles.bold), } } -/// Write `METHOD /route` for an endpoint base path. +/// Append `METHOD /route` for an endpoint base path. /// /// `name` is assumed to match the `paths//` shape that /// [`DocumentBasePath::classify`] enforces on `Endpoint` variants. -fn write_endpoint_method_route( - f: &mut fmt::Formatter<'_>, - name: &DocumentPath, +fn append_endpoint_method_route<'a>( + line: &mut Line<'a>, + name: &'a DocumentPath, styles: &Styles, -) -> fmt::Result { +) { // We expect this to be `paths//`: // // * segments[1] is the unescaped route, // * segments[2] is the HTTP method. - let route = &name.segments[1]; - let method = &name.segments[2]; - write!( - f, - "{} {}", - method.to_uppercase().style(styles.success_header), - route.style(styles.filename), - ) + let route = name.segments[1].as_str(); + let method = name.segments[2].to_uppercase(); + line.push(method, styles.success_header) + .push_plain(" ") + .push(route, styles.filename); } -/// Append `" ()"`, or nothing if `operation_id` is `None`. -fn write_operation_id( - f: &mut fmt::Formatter<'_>, - operation_id: Option<&str>, +/// Append ` ()` to `line` in the operation-id style, or no-op if +/// `op_id` is `None`. A leading space is included so callers can call +/// this unconditionally. The trio is split across three spans only for +/// styling — no internal whitespace means a wrap can only occur before +/// the leading space, not inside `(op_id)`. +fn append_operation_id<'a>( + line: &mut Line<'a>, + op_id: Option<&'a str>, styles: &Styles, -) -> fmt::Result { - let Some(op_id) = operation_id else { return Ok(()) }; - write!(f, " ({})", op_id.style(styles.operation_id)) +) { + let Some(op_id) = op_id else { return }; + line.push_plain(" (").push(op_id, styles.operation_id).push_plain(")"); } -/// Write a component's subpath in OpenAPI-aware form. +/// Append a component's subpath in OpenAPI-aware form. /// /// With OpenAPI, paths can contain both structural keywords (`properties`, /// `items`, etc.) and field names. The structural pieces do not add much value, @@ -575,59 +639,57 @@ fn write_operation_id( /// /// Other keywords (`allOf.0`, `additionalProperties`, etc.) are /// rendered literally. -fn write_humanized_subpath( - f: &mut fmt::Formatter<'_>, - subpath: &DocumentPath, +fn append_humanized_subpath<'a>( + line: &mut Line<'a>, + subpath: &'a DocumentPath, style: Style, -) -> fmt::Result { +) { let mut next_is_field_name = false; for s in &subpath.segments { if next_is_field_name { - write_jq_segment(f, s, style)?; + append_jq_segment(line, s, style); next_is_field_name = false; continue; } match s.as_str() { "properties" => next_is_field_name = true, - "items" => write!(f, "{}", "[]".style(style))?, - other => write_jq_segment(f, other, style)?, + "items" => { + line.push("[]", style); + } + other => append_jq_segment(line, other, style), } } - Ok(()) } -/// Write `path` in plain jq notation, applying `style` to every segment. +/// Append `path` to `line` in plain jq notation, applying `style` to every +/// segment. /// /// The root (empty) path renders as `.`, while non-empty paths render as /// `.seg1.seg2…`. -fn write_jq_path( - f: &mut fmt::Formatter<'_>, - path: &DocumentPath, +fn append_jq_path<'a>( + line: &mut Line<'a>, + path: &'a DocumentPath, style: Style, -) -> fmt::Result { +) { if path.is_root() { - return write!(f, "{}", ".".style(style)); + line.push(".", style); + return; } for s in &path.segments { - write_jq_segment(f, s, style)?; + append_jq_segment(line, s, style); } - Ok(()) } -/// Write one jq segment starting with a leading `.`. +/// Append one jq segment to `line` starting with a leading `.`. /// /// The segment text is quoted if it contains characters ambiguous as a bare jq /// identifier. -fn write_jq_segment( - f: &mut fmt::Formatter<'_>, - segment: &str, - style: Style, -) -> fmt::Result { - write!(f, "{}", ".".style(style))?; +fn append_jq_segment<'a>(line: &mut Line<'a>, segment: &'a str, style: Style) { + line.push(".", style); if segment.contains(['/', '~']) { - write!(f, "{}", format_args!("\"{segment}\"").style(style)) + line.push(format!("\"{segment}\""), style); } else { - write!(f, "{}", segment.style(style)) + line.push(segment, style); } } @@ -736,12 +798,14 @@ mod tests { ); } - /// Render `path` unstyled into a `String` using [`write_jq_path`]. + /// Render `path` unstyled into a `String` using [`append_jq_path`]. fn jq_string(path: &DocumentPath) -> String { struct Render<'a>(&'a DocumentPath); impl fmt::Display for Render<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write_jq_path(f, self.0, Style::default()) + let mut line = Line::new(); + append_jq_path(&mut line, self.0, Style::default()); + line.write_inline(f) } } Render(path).to_string() @@ -1220,6 +1284,73 @@ mod tests { ); } + #[test] + fn test_render_wrapped_long_endpoint() { + // A long, realistic Oxide-style endpoint name with multiple path + // parameters that would normally exceed terminal width when prefixed by + // `used by: GET `. + let endpoint = "#/paths/~1v1~1instances~1{instance}\ + ~1network-interfaces~1{network_interface}~1multicast/post"; + let ops = op_ids(&[(endpoint, "instance_network_multicast_attach")]); + let make_chain = || { + vec![ + PathTreeKey::parse( + "#/components/schemas/MulticastGroupMember\ + /properties/identity/$ref", + &ops, + ), + PathTreeKey::parse( + &format!( + "{}/responses/200/content/application~1json/schema/$ref", + endpoint, + ), + &ops, + ), + ] + }; + let mut tree = PathTree::default(); + tree.insert(make_chain()); + + let issue = ApiCompatIssue { + blessed_base: component( + "#/components/schemas/MulticastGroupIdentity", + ), + generated_base: component( + "#/components/schemas/MulticastGroupIdentity", + ), + changes: BTreeSet::from([SubpathChange { + class: ChangeClass::Incompatible, + message: "schema types changed".into(), + old_subpath: DocumentPath::parse("properties/id"), + new_subpath: DocumentPath::parse("properties/id"), + }]), + tree, + blessed_value: None, + generated_value: None, + }; + + // Pin the width so the test doesn't depend on the developer's + // terminal size. 80 is a typical width where the `used by:` + // endpoint overflows but the rest of the block doesn't. + let mut colorized = Styles::default(); + colorized.colorize(); + let status = CompatRenderStatus::FirstOccurrence { anchor: None }; + expectorate::assert_contents( + output_path("wrapped_long_endpoint.txt"), + &format!( + "{}\n", + issue.display(&Styles::default(), status).with_wrap_width(80), + ), + ); + expectorate::assert_contents( + output_path("wrapped_long_endpoint.ansi"), + &format!( + "{}\n", + issue.display(&colorized, status).with_wrap_width(80), + ), + ); + } + /// Snapshot the issue in its surrounding render context: a cargo-style /// `Failure` header, the second-tier `error:` keyword, and the issue /// body indented so its leftmost label aligns with where `error` diff --git a/crates/dropshot-api-manager/src/compatibility/mod.rs b/crates/dropshot-api-manager/src/compatibility/mod.rs index 842d808..d137dab 100644 --- a/crates/dropshot-api-manager/src/compatibility/mod.rs +++ b/crates/dropshot-api-manager/src/compatibility/mod.rs @@ -5,6 +5,7 @@ mod detect; mod display; mod types; +mod wrap; pub(crate) use detect::{ CompatDedupMap, FinalizedCompatDedupMap, api_compatible, diff --git a/crates/dropshot-api-manager/src/compatibility/types.rs b/crates/dropshot-api-manager/src/compatibility/types.rs index 9d991b6..3ebc8ed 100644 --- a/crates/dropshot-api-manager/src/compatibility/types.rs +++ b/crates/dropshot-api-manager/src/compatibility/types.rs @@ -55,12 +55,15 @@ impl ApiCompatIssue { } /// Returns a `Display` adapter that renders this issue per `status`. + /// + /// The returned adapter does not perform any wrapping by default. Call + /// [`ApiCompatIssueDisplay::with_wrap_width`] to wrap long lines. pub(crate) fn display<'a>( &'a self, styles: &'a Styles, status: CompatRenderStatus, ) -> ApiCompatIssueDisplay<'a> { - ApiCompatIssueDisplay { issue: self, styles, status } + ApiCompatIssueDisplay { issue: self, styles, status, wrap_width: None } } /// Returns true if `self` and `other` represent the same underlying diff --git a/crates/dropshot-api-manager/src/compatibility/wrap.rs b/crates/dropshot-api-manager/src/compatibility/wrap.rs new file mode 100644 index 0000000..533f3ad --- /dev/null +++ b/crates/dropshot-api-manager/src/compatibility/wrap.rs @@ -0,0 +1,317 @@ +// Copyright 2026 Oxide Computer Company + +//! Style-aware text wrapping for compatibility output. +//! +//! The [`super::display`] layer builds each "logical line" of an issue +//! header (the content after `at:` or `used by:`) as a [`Line`] of styled +//! [`Span`]s, then hands it to [`write_wrapped`] to lay out at a target +//! width. +//! +//! Wrapping is only performed at ASCII spaces between words. A "word" is a +//! contiguous run of non-space characters across one or more spans. Words wider +//! than the line extend past the boundary rather than being broken: endpoint +//! paths like `/v1/instances/{instance}/…` need to remain copyable from the +//! terminal as a single unit. +//! +//! Adapted from `wicket/src/ui/wrap.rs` in oxidecomputer/omicron. + +use owo_colors::{OwoColorize, Style}; +use std::{ + borrow::Cow, + fmt::{self, Write as _}, +}; +use textwrap::core::display_width; + +#[derive(Clone, Debug)] +pub(super) struct Span<'a> { + content: Cow<'a, str>, + style: Style, +} + +#[derive(Debug, Default)] +pub(super) struct Line<'a> { + spans: Vec>, +} + +impl<'a> Line<'a> { + pub(super) fn new() -> Self { + Self::default() + } + + /// Append a styled span. + pub(super) fn push( + &mut self, + content: impl Into>, + style: Style, + ) -> &mut Self { + let content = content.into(); + if !content.is_empty() { + self.spans.push(Span { content, style }); + } + self + } + + /// Append an unstyled span. + /// + /// Shorthand for `push(content, Style::default())`. + pub(super) fn push_plain( + &mut self, + content: impl Into>, + ) -> &mut Self { + self.push(content, Style::default()) + } + + /// Write every span to `f` with its style applied, with no wrapping. + /// Useful inside tree-drawn rows where the surrounding scaffolding + /// already constrains the layout. + pub(super) fn write_inline( + &self, + f: &mut fmt::Formatter<'_>, + ) -> fmt::Result { + for span in &self.spans { + write!(f, "{}", span.content.as_ref().style(span.style))?; + } + Ok(()) + } +} + +/// A subsequent-line indent: the literal string emitted at the start +/// of each continuation line, paired with its visible column width. +/// +/// Width is supplied by the caller rather than measured so +/// [`write_wrapped`] makes no assumption about the indent's content. +/// Plain space indents (the only kind in use today) are constructed +/// via [`Self::spaces`]. +#[derive(Clone, Copy, Debug)] +pub(super) struct Indent<'a> { + pub(super) string: &'a str, + pub(super) width: usize, +} + +impl<'a> Indent<'a> { + /// Construct an `Indent` from a string of ASCII spaces. + pub(super) fn spaces(string: &'a str) -> Self { + debug_assert!( + string.bytes().all(|b| b == b' '), + "Indent::spaces called with non-space content: {string:?}", + ); + Self { string, width: string.len() } + } +} + +/// A wrap-time unit: a sequence of styled body slices that move together +/// across line breaks, plus the trailing space run that follows them in +/// the source content. +/// +/// `body_width` is the visible width of the body (for fit decisions); +/// `trailing_ws_width` is the width of the trailing space run, dropped +/// when this word is the last on a wrapped line. +#[derive(Debug, Default)] +struct StyledWord<'a> { + body: Vec<(&'a str, Style)>, + body_width: usize, + trailing_ws_width: usize, +} + +impl StyledWord<'_> { + fn is_empty(&self) -> bool { + self.body.is_empty() && self.trailing_ws_width == 0 + } +} + +/// Walk `line`'s spans and split them into [`StyledWord`]s at ASCII-space +/// boundaries. Non-space content from consecutive spans merges into a +/// single word so style transitions within a word (e.g., `(` default → +/// `op_id` purple → `)` default) don't introduce break candidates. +fn collect_words<'a>(line: &'a Line<'a>) -> Vec> { + let mut words = Vec::new(); + let mut current = StyledWord::default(); + // True once we've moved past the body of the current word into its + // trailing whitespace run. The next non-space character commits the + // current word and starts a new one. + let mut in_trailing_ws = false; + + for span in &line.spans { + let mut body_start = 0; + let content = span.content.as_ref(); + let bytes = content.as_bytes(); + let mut i = 0; + while i < bytes.len() { + if bytes[i] == b' ' { + if !in_trailing_ws { + // Closing out the body portion of this span: commit + // the slice [body_start, i) to the current word body + // before we start counting trailing whitespace. + if body_start < i { + let slice = &content[body_start..i]; + current.body.push((slice, span.style)); + current.body_width += display_width(slice); + } + in_trailing_ws = true; + } + current.trailing_ws_width += 1; + i += 1; + } else { + if in_trailing_ws { + // Trailing whitespace just ended — commit the current + // word and start a fresh one. The non-space character + // we just saw is the first byte of the new word's body. + words.push(std::mem::take(&mut current)); + in_trailing_ws = false; + body_start = i; + } + i += 1; + } + } + // Anything left over in this span goes into the current bucket: + // body if we're not in trailing whitespace yet, otherwise the + // trailing run has already been counted above. + if !in_trailing_ws && body_start < bytes.len() { + let slice = &content[body_start..]; + current.body.push((slice, span.style)); + current.body_width += display_width(slice); + } + } + if !current.is_empty() { + words.push(current); + } + words +} + +/// Write `line` to `f`, wrapping at `width` columns. +/// +/// `width` is the total visible width available; on continuation lines +/// we emit `indent.string` first and the remaining `width - indent.width` +/// columns are available for content. The first line is assumed to +/// already be positioned at column `indent.width` by the caller +/// (typically by writing a right-aligned label of the same visible +/// width), so the same content width applies to every line of the block. +/// +/// Single words that overflow extend past width. The alternative would defeat +/// the user's ability to copy it from the terminal in one piece (particularly +/// for long endpoint paths). +pub(super) fn write_wrapped( + f: &mut fmt::Formatter<'_>, + line: &Line<'_>, + width: usize, + indent: Indent<'_>, +) -> fmt::Result { + let words = collect_words(line); + if words.is_empty() { + return Ok(()); + } + + let content_width = width.saturating_sub(indent.width); + + // Greedy first-fit: a word joins the current line if its body still + // fits, otherwise it starts a new line. Width is measured against + // body alone (not body + trailing whitespace) so a trailing space + // that would overflow doesn't force a premature break — it just + // gets dropped at the line end. + let mut line_ranges: Vec<(usize, usize)> = Vec::new(); + let mut start = 0; + let mut current_width = 0; + for (i, word) in words.iter().enumerate() { + let cant_fit_here = current_width > 0 + && current_width + word.body_width > content_width; + if cant_fit_here { + line_ranges.push((start, i)); + start = i; + current_width = 0; + } + current_width += word.body_width + word.trailing_ws_width; + } + line_ranges.push((start, words.len())); + + for (line_idx, (lo, hi)) in line_ranges.iter().copied().enumerate() { + if line_idx > 0 { + writeln!(f)?; + f.write_str(indent.string)?; + } + let group = &words[lo..hi]; + let last_idx = group.len().saturating_sub(1); + for (j, word) in group.iter().enumerate() { + for (slice, style) in &word.body { + write!(f, "{}", slice.style(*style))?; + } + // Drop trailing whitespace at the end of a wrapped line so we + // don't emit dangling spaces; preserve it between words on the + // same output line. + if j < last_idx && word.trailing_ws_width > 0 { + for _ in 0..word.trailing_ws_width { + f.write_char(' ')?; + } + } + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Render `line` at the given `width` with an indent of `indent` + /// ASCII spaces, returning the resulting string. + fn render(line: &Line<'_>, width: usize, indent: &str) -> String { + struct Adapter<'a> { + line: &'a Line<'a>, + width: usize, + indent: Indent<'a>, + } + impl fmt::Display for Adapter<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write_wrapped(f, self.line, self.width, self.indent) + } + } + Adapter { line, width, indent: Indent::spaces(indent) }.to_string() + } + + #[test] + fn test_fit_on_one_line() { + let mut line = Line::new(); + line.push_plain("GET ").push_plain("/short"); + assert_eq!(render(&line, 80, " "), "GET /short"); + } + + #[test] + fn test_break_at_whitespace() { + let mut line = Line::new(); + line.push_plain("alpha beta"); + // Width 7: "alpha" (5) fits, "beta" (4) won't fit alongside it + // (5+1+4=10>7) and fits fine on a line of its own, so it + // breaks. + assert_eq!(render(&line, 7, " "), "alpha\n beta"); + } + + #[test] + fn test_adjacent_spans_form_one_word() { + let mut line = Line::new(); + line.push_plain("AB").push_plain("CD"); + // "ABCD" is 4 chars; at width 3, a naive per-span breaker would + // split between "AB" and "CD" — but they form one word here, so + // the whole thing extends past the limit on a single line. + assert_eq!(render(&line, 3, " "), "ABCD"); + } + + #[test] + fn test_long_word_overflows() { + let mut line = Line::new(); + line.push_plain("a ").push_plain("loooooooooong ").push_plain("z"); + // Width 5: each word goes on its own line ("a" fits, the long + // word overflows alone, "z" fits). + assert_eq!(render(&line, 5, ""), "a\nloooooooooong\nz"); + } + + #[test] + fn test_preserve_styles_across_wrap() { + let bold = Style::new().bold(); + let mut line = Line::new(); + line.push_plain("aa bb ").push("CC", bold).push_plain("-dd"); + // Width 7 unindented: "aa bb" (5) fits on line 1; "CC-dd" (5) + // doesn't fit alongside it (5+1+5=11>7) and starts line 2 with + // the bold styling on CC preserved. + let expected = format!("aa bb\n{}-dd", "CC".style(bold)); + assert_eq!(render(&line, 7, ""), expected); + } +} diff --git a/crates/dropshot-api-manager/src/output.rs b/crates/dropshot-api-manager/src/output.rs index e119c33..55ede79 100644 --- a/crates/dropshot-api-manager/src/output.rs +++ b/crates/dropshot-api-manager/src/output.rs @@ -767,13 +767,18 @@ fn display_compat_issue( // already ends in a newline). writeln!(writer)?; + // Wrap at terminal width minus the body indent. (`display_width` matches + // what `wrap.rs` uses for its own indent.) + let wrap_width = + term_width().saturating_sub(textwrap::core::display_width(body_indent)); + // Indent every line of the rendered block. `IndentWriter` prefixes the // first line as well, so we don't need a separate initial-indent string. let mut buf = String::new(); write!( IndentWriter::new(body_indent, &mut buf), "{}", - issue.display(styles, status), + issue.display(styles, status).with_wrap_width(wrap_width), ) .expect("writing to a String never fails"); writeln!(writer, "{buf}")?; diff --git a/crates/dropshot-api-manager/tests/output/compatibility/wrapped_long_endpoint.ansi b/crates/dropshot-api-manager/tests/output/compatibility/wrapped_long_endpoint.ansi new file mode 100644 index 0000000..b856506 --- /dev/null +++ b/crates/dropshot-api-manager/tests/output/compatibility/wrapped_long_endpoint.ansi @@ -0,0 +1,6 @@ +incompatible: schema types changed + at: MulticastGroupIdentity.id + used by: POST + /v1/instances/{instance}/network-interfaces/{network_interface}/multicast + (instance_network_multicast_attach) + └─ MulticastGroupMember.identity diff --git a/crates/dropshot-api-manager/tests/output/compatibility/wrapped_long_endpoint.txt b/crates/dropshot-api-manager/tests/output/compatibility/wrapped_long_endpoint.txt new file mode 100644 index 0000000..0df8c5e --- /dev/null +++ b/crates/dropshot-api-manager/tests/output/compatibility/wrapped_long_endpoint.txt @@ -0,0 +1,6 @@ +incompatible: schema types changed + at: MulticastGroupIdentity.id + used by: POST + /v1/instances/{instance}/network-interfaces/{network_interface}/multicast + (instance_network_multicast_attach) + └─ MulticastGroupMember.identity diff --git a/crates/integration-tests/tests/output/integration/blessed_version_broken.txt b/crates/integration-tests/tests/output/integration/blessed_version_broken.txt index 0b4f3f0..488411e 100644 --- a/crates/integration-tests/tests/output/integration/blessed_version_broken.txt +++ b/crates/integration-tests/tests/output/integration/blessed_version_broken.txt @@ -10,7 +10,8 @@ with the blessed document (from upstream) backward-incompatible: A new, required parameter 'verbose' was added [#1] - at: GET /health/detailed (detailed_health_check) (.parameters.0) + at: GET /health/detailed (detailed_health_check) + (.parameters.0) --- a/blessed +++ b/generated @@ -1,11 +1,22 @@ @@ -41,8 +42,10 @@ error: OpenAPI document generated from the current code is not compatible with the blessed document (from upstream) - backward-incompatible: A new, required parameter 'verbose' was added (see #1) - at: GET /health/detailed (detailed_health_check) (.parameters.0) + backward-incompatible: A new, required parameter 'verbose' was added (see + #1) + at: GET /health/detailed (detailed_health_check) + (.parameters.0) forward-incompatible: The operation get_system_info was added at: GET /system/info (get_system_info) diff --git a/crates/integration-tests/tests/output/integration/blessed_version_endpoint_removed.txt b/crates/integration-tests/tests/output/integration/blessed_version_endpoint_removed.txt index e9242d0..7c0d522 100644 --- a/crates/integration-tests/tests/output/integration/blessed_version_endpoint_removed.txt +++ b/crates/integration-tests/tests/output/integration/blessed_version_endpoint_removed.txt @@ -10,7 +10,8 @@ with the blessed document (from upstream) backward-incompatible: The parameter 'verbose' was removed [#1] - at: GET /health/detailed (detailed_health_check) (.parameters.0) + at: GET /health/detailed (detailed_health_check) + (.parameters.0) --- a/blessed +++ b/generated @@ -1,22 +1,11 @@ @@ -42,7 +43,8 @@ with the blessed document (from upstream) backward-incompatible: The parameter 'verbose' was removed (see #1) - at: GET /health/detailed (detailed_health_check) (.parameters.0) + at: GET /health/detailed (detailed_health_check) + (.parameters.0) backward-incompatible: The operation get_system_info was removed at: GET /system/info (get_system_info) diff --git a/crates/integration-tests/tests/output/integration/cross_api_dedup.txt b/crates/integration-tests/tests/output/integration/cross_api_dedup.txt index 772aaed..6e97c62 100644 --- a/crates/integration-tests/tests/output/integration/cross_api_dedup.txt +++ b/crates/integration-tests/tests/output/integration/cross_api_dedup.txt @@ -10,7 +10,8 @@ with the blessed document (from upstream) backward-incompatible: A new, required parameter 'verbose' was added [#1] - at: GET /health/detailed (detailed_health_check) (.parameters.0) + at: GET /health/detailed (detailed_health_check) + (.parameters.0) --- a/blessed +++ b/generated @@ -1,11 +1,22 @@ @@ -41,8 +42,10 @@ error: OpenAPI document generated from the current code is not compatible with the blessed document (from upstream) - backward-incompatible: A new, required parameter 'verbose' was added (see #1) - at: GET /health/detailed (detailed_health_check) (.parameters.0) + backward-incompatible: A new, required parameter 'verbose' was added (see + #1) + at: GET /health/detailed (detailed_health_check) + (.parameters.0) forward-incompatible: The operation get_system_info was added [#2] at: GET /system/info (get_system_info) @@ -81,15 +84,19 @@ error: OpenAPI document generated from the current code is not compatible with the blessed document (from upstream) - backward-incompatible: A new, required parameter 'verbose' was added (see #1) - at: GET /health/detailed (detailed_health_check) (.parameters.0) + backward-incompatible: A new, required parameter 'verbose' was added (see + #1) + at: GET /health/detailed (detailed_health_check) + (.parameters.0) Failure versioned-monitor (versioned v3.0.0 (blessed)): Versioned Monitor API error: OpenAPI document generated from the current code is not compatible with the blessed document (from upstream) - backward-incompatible: A new, required parameter 'verbose' was added (see #1) - at: GET /health/detailed (detailed_health_check) (.parameters.0) + backward-incompatible: A new, required parameter 'verbose' was added (see + #1) + at: GET /health/detailed (detailed_health_check) + (.parameters.0) forward-incompatible: The operation get_system_info was added (see #2) at: GET /system/info (get_system_info)