Skip to content

Commit f7fb3ae

Browse files
authored
[3/n] [dropshot-api-manager] wrap long lines in compatibility output
The rest of the API manager wraps text properly, so let's do that in compatibility output as well. <img width="1188" height="331" alt="image" src="https://github.com/user-attachments/assets/7b425194-c23c-4016-8122-b550f9e5ee1b" /> I skipped over trying to implement wrapping for reference trees -- hopefully it won't be necessary, because it is a bit of a pain to do. Reviewers: david-crespo Reviewed By: david-crespo Pull Request: #105
1 parent b2c77ea commit f7fb3ae

10 files changed

Lines changed: 613 additions & 132 deletions

File tree

crates/dropshot-api-manager/src/compatibility/display.rs

Lines changed: 249 additions & 118 deletions
Large diffs are not rendered by default.

crates/dropshot-api-manager/src/compatibility/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
mod detect;
66
mod display;
77
mod types;
8+
mod wrap;
89

910
pub(crate) use detect::{
1011
CompatDedupMap, FinalizedCompatDedupMap, api_compatible,

crates/dropshot-api-manager/src/compatibility/types.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,15 @@ impl ApiCompatIssue {
5555
}
5656

5757
/// Returns a `Display` adapter that renders this issue per `status`.
58+
///
59+
/// The returned adapter does not perform any wrapping by default. Call
60+
/// [`ApiCompatIssueDisplay::with_wrap_width`] to wrap long lines.
5861
pub(crate) fn display<'a>(
5962
&'a self,
6063
styles: &'a Styles,
6164
status: CompatRenderStatus,
6265
) -> ApiCompatIssueDisplay<'a> {
63-
ApiCompatIssueDisplay { issue: self, styles, status }
66+
ApiCompatIssueDisplay { issue: self, styles, status, wrap_width: None }
6467
}
6568

6669
/// Returns true if `self` and `other` represent the same underlying
Lines changed: 317 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,317 @@
1+
// Copyright 2026 Oxide Computer Company
2+
3+
//! Style-aware text wrapping for compatibility output.
4+
//!
5+
//! The [`super::display`] layer builds each "logical line" of an issue
6+
//! header (the content after `at:` or `used by:`) as a [`Line`] of styled
7+
//! [`Span`]s, then hands it to [`write_wrapped`] to lay out at a target
8+
//! width.
9+
//!
10+
//! Wrapping is only performed at ASCII spaces between words. A "word" is a
11+
//! contiguous run of non-space characters across one or more spans. Words wider
12+
//! than the line extend past the boundary rather than being broken: endpoint
13+
//! paths like `/v1/instances/{instance}/…` need to remain copyable from the
14+
//! terminal as a single unit.
15+
//!
16+
//! Adapted from `wicket/src/ui/wrap.rs` in oxidecomputer/omicron.
17+
18+
use owo_colors::{OwoColorize, Style};
19+
use std::{
20+
borrow::Cow,
21+
fmt::{self, Write as _},
22+
};
23+
use textwrap::core::display_width;
24+
25+
#[derive(Clone, Debug)]
26+
pub(super) struct Span<'a> {
27+
content: Cow<'a, str>,
28+
style: Style,
29+
}
30+
31+
#[derive(Debug, Default)]
32+
pub(super) struct Line<'a> {
33+
spans: Vec<Span<'a>>,
34+
}
35+
36+
impl<'a> Line<'a> {
37+
pub(super) fn new() -> Self {
38+
Self::default()
39+
}
40+
41+
/// Append a styled span.
42+
pub(super) fn push(
43+
&mut self,
44+
content: impl Into<Cow<'a, str>>,
45+
style: Style,
46+
) -> &mut Self {
47+
let content = content.into();
48+
if !content.is_empty() {
49+
self.spans.push(Span { content, style });
50+
}
51+
self
52+
}
53+
54+
/// Append an unstyled span.
55+
///
56+
/// Shorthand for `push(content, Style::default())`.
57+
pub(super) fn push_plain(
58+
&mut self,
59+
content: impl Into<Cow<'a, str>>,
60+
) -> &mut Self {
61+
self.push(content, Style::default())
62+
}
63+
64+
/// Write every span to `f` with its style applied, with no wrapping.
65+
/// Useful inside tree-drawn rows where the surrounding scaffolding
66+
/// already constrains the layout.
67+
pub(super) fn write_inline(
68+
&self,
69+
f: &mut fmt::Formatter<'_>,
70+
) -> fmt::Result {
71+
for span in &self.spans {
72+
write!(f, "{}", span.content.as_ref().style(span.style))?;
73+
}
74+
Ok(())
75+
}
76+
}
77+
78+
/// A subsequent-line indent: the literal string emitted at the start
79+
/// of each continuation line, paired with its visible column width.
80+
///
81+
/// Width is supplied by the caller rather than measured so
82+
/// [`write_wrapped`] makes no assumption about the indent's content.
83+
/// Plain space indents (the only kind in use today) are constructed
84+
/// via [`Self::spaces`].
85+
#[derive(Clone, Copy, Debug)]
86+
pub(super) struct Indent<'a> {
87+
pub(super) string: &'a str,
88+
pub(super) width: usize,
89+
}
90+
91+
impl<'a> Indent<'a> {
92+
/// Construct an `Indent` from a string of ASCII spaces.
93+
pub(super) fn spaces(string: &'a str) -> Self {
94+
debug_assert!(
95+
string.bytes().all(|b| b == b' '),
96+
"Indent::spaces called with non-space content: {string:?}",
97+
);
98+
Self { string, width: string.len() }
99+
}
100+
}
101+
102+
/// A wrap-time unit: a sequence of styled body slices that move together
103+
/// across line breaks, plus the trailing space run that follows them in
104+
/// the source content.
105+
///
106+
/// `body_width` is the visible width of the body (for fit decisions);
107+
/// `trailing_ws_width` is the width of the trailing space run, dropped
108+
/// when this word is the last on a wrapped line.
109+
#[derive(Debug, Default)]
110+
struct StyledWord<'a> {
111+
body: Vec<(&'a str, Style)>,
112+
body_width: usize,
113+
trailing_ws_width: usize,
114+
}
115+
116+
impl StyledWord<'_> {
117+
fn is_empty(&self) -> bool {
118+
self.body.is_empty() && self.trailing_ws_width == 0
119+
}
120+
}
121+
122+
/// Walk `line`'s spans and split them into [`StyledWord`]s at ASCII-space
123+
/// boundaries. Non-space content from consecutive spans merges into a
124+
/// single word so style transitions within a word (e.g., `(` default →
125+
/// `op_id` purple → `)` default) don't introduce break candidates.
126+
fn collect_words<'a>(line: &'a Line<'a>) -> Vec<StyledWord<'a>> {
127+
let mut words = Vec::new();
128+
let mut current = StyledWord::default();
129+
// True once we've moved past the body of the current word into its
130+
// trailing whitespace run. The next non-space character commits the
131+
// current word and starts a new one.
132+
let mut in_trailing_ws = false;
133+
134+
for span in &line.spans {
135+
let mut body_start = 0;
136+
let content = span.content.as_ref();
137+
let bytes = content.as_bytes();
138+
let mut i = 0;
139+
while i < bytes.len() {
140+
if bytes[i] == b' ' {
141+
if !in_trailing_ws {
142+
// Closing out the body portion of this span: commit
143+
// the slice [body_start, i) to the current word body
144+
// before we start counting trailing whitespace.
145+
if body_start < i {
146+
let slice = &content[body_start..i];
147+
current.body.push((slice, span.style));
148+
current.body_width += display_width(slice);
149+
}
150+
in_trailing_ws = true;
151+
}
152+
current.trailing_ws_width += 1;
153+
i += 1;
154+
} else {
155+
if in_trailing_ws {
156+
// Trailing whitespace just ended — commit the current
157+
// word and start a fresh one. The non-space character
158+
// we just saw is the first byte of the new word's body.
159+
words.push(std::mem::take(&mut current));
160+
in_trailing_ws = false;
161+
body_start = i;
162+
}
163+
i += 1;
164+
}
165+
}
166+
// Anything left over in this span goes into the current bucket:
167+
// body if we're not in trailing whitespace yet, otherwise the
168+
// trailing run has already been counted above.
169+
if !in_trailing_ws && body_start < bytes.len() {
170+
let slice = &content[body_start..];
171+
current.body.push((slice, span.style));
172+
current.body_width += display_width(slice);
173+
}
174+
}
175+
if !current.is_empty() {
176+
words.push(current);
177+
}
178+
words
179+
}
180+
181+
/// Write `line` to `f`, wrapping at `width` columns.
182+
///
183+
/// `width` is the total visible width available; on continuation lines
184+
/// we emit `indent.string` first and the remaining `width - indent.width`
185+
/// columns are available for content. The first line is assumed to
186+
/// already be positioned at column `indent.width` by the caller
187+
/// (typically by writing a right-aligned label of the same visible
188+
/// width), so the same content width applies to every line of the block.
189+
///
190+
/// Single words that overflow extend past width. The alternative would defeat
191+
/// the user's ability to copy it from the terminal in one piece (particularly
192+
/// for long endpoint paths).
193+
pub(super) fn write_wrapped(
194+
f: &mut fmt::Formatter<'_>,
195+
line: &Line<'_>,
196+
width: usize,
197+
indent: Indent<'_>,
198+
) -> fmt::Result {
199+
let words = collect_words(line);
200+
if words.is_empty() {
201+
return Ok(());
202+
}
203+
204+
let content_width = width.saturating_sub(indent.width);
205+
206+
// Greedy first-fit: a word joins the current line if its body still
207+
// fits, otherwise it starts a new line. Width is measured against
208+
// body alone (not body + trailing whitespace) so a trailing space
209+
// that would overflow doesn't force a premature break — it just
210+
// gets dropped at the line end.
211+
let mut line_ranges: Vec<(usize, usize)> = Vec::new();
212+
let mut start = 0;
213+
let mut current_width = 0;
214+
for (i, word) in words.iter().enumerate() {
215+
let cant_fit_here = current_width > 0
216+
&& current_width + word.body_width > content_width;
217+
if cant_fit_here {
218+
line_ranges.push((start, i));
219+
start = i;
220+
current_width = 0;
221+
}
222+
current_width += word.body_width + word.trailing_ws_width;
223+
}
224+
line_ranges.push((start, words.len()));
225+
226+
for (line_idx, (lo, hi)) in line_ranges.iter().copied().enumerate() {
227+
if line_idx > 0 {
228+
writeln!(f)?;
229+
f.write_str(indent.string)?;
230+
}
231+
let group = &words[lo..hi];
232+
let last_idx = group.len().saturating_sub(1);
233+
for (j, word) in group.iter().enumerate() {
234+
for (slice, style) in &word.body {
235+
write!(f, "{}", slice.style(*style))?;
236+
}
237+
// Drop trailing whitespace at the end of a wrapped line so we
238+
// don't emit dangling spaces; preserve it between words on the
239+
// same output line.
240+
if j < last_idx && word.trailing_ws_width > 0 {
241+
for _ in 0..word.trailing_ws_width {
242+
f.write_char(' ')?;
243+
}
244+
}
245+
}
246+
}
247+
Ok(())
248+
}
249+
250+
#[cfg(test)]
251+
mod tests {
252+
use super::*;
253+
254+
/// Render `line` at the given `width` with an indent of `indent`
255+
/// ASCII spaces, returning the resulting string.
256+
fn render(line: &Line<'_>, width: usize, indent: &str) -> String {
257+
struct Adapter<'a> {
258+
line: &'a Line<'a>,
259+
width: usize,
260+
indent: Indent<'a>,
261+
}
262+
impl fmt::Display for Adapter<'_> {
263+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
264+
write_wrapped(f, self.line, self.width, self.indent)
265+
}
266+
}
267+
Adapter { line, width, indent: Indent::spaces(indent) }.to_string()
268+
}
269+
270+
#[test]
271+
fn test_fit_on_one_line() {
272+
let mut line = Line::new();
273+
line.push_plain("GET ").push_plain("/short");
274+
assert_eq!(render(&line, 80, " "), "GET /short");
275+
}
276+
277+
#[test]
278+
fn test_break_at_whitespace() {
279+
let mut line = Line::new();
280+
line.push_plain("alpha beta");
281+
// Width 7: "alpha" (5) fits, "beta" (4) won't fit alongside it
282+
// (5+1+4=10>7) and fits fine on a line of its own, so it
283+
// breaks.
284+
assert_eq!(render(&line, 7, " "), "alpha\n beta");
285+
}
286+
287+
#[test]
288+
fn test_adjacent_spans_form_one_word() {
289+
let mut line = Line::new();
290+
line.push_plain("AB").push_plain("CD");
291+
// "ABCD" is 4 chars; at width 3, a naive per-span breaker would
292+
// split between "AB" and "CD" — but they form one word here, so
293+
// the whole thing extends past the limit on a single line.
294+
assert_eq!(render(&line, 3, " "), "ABCD");
295+
}
296+
297+
#[test]
298+
fn test_long_word_overflows() {
299+
let mut line = Line::new();
300+
line.push_plain("a ").push_plain("loooooooooong ").push_plain("z");
301+
// Width 5: each word goes on its own line ("a" fits, the long
302+
// word overflows alone, "z" fits).
303+
assert_eq!(render(&line, 5, ""), "a\nloooooooooong\nz");
304+
}
305+
306+
#[test]
307+
fn test_preserve_styles_across_wrap() {
308+
let bold = Style::new().bold();
309+
let mut line = Line::new();
310+
line.push_plain("aa bb ").push("CC", bold).push_plain("-dd");
311+
// Width 7 unindented: "aa bb" (5) fits on line 1; "CC-dd" (5)
312+
// doesn't fit alongside it (5+1+5=11>7) and starts line 2 with
313+
// the bold styling on CC preserved.
314+
let expected = format!("aa bb\n{}-dd", "CC".style(bold));
315+
assert_eq!(render(&line, 7, ""), expected);
316+
}
317+
}

crates/dropshot-api-manager/src/output.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -767,13 +767,18 @@ fn display_compat_issue(
767767
// already ends in a newline).
768768
writeln!(writer)?;
769769

770+
// Wrap at terminal width minus the body indent. (`display_width` matches
771+
// what `wrap.rs` uses for its own indent.)
772+
let wrap_width =
773+
term_width().saturating_sub(textwrap::core::display_width(body_indent));
774+
770775
// Indent every line of the rendered block. `IndentWriter` prefixes the
771776
// first line as well, so we don't need a separate initial-indent string.
772777
let mut buf = String::new();
773778
write!(
774779
IndentWriter::new(body_indent, &mut buf),
775780
"{}",
776-
issue.display(styles, status),
781+
issue.display(styles, status).with_wrap_width(wrap_width),
777782
)
778783
.expect("writing to a String never fails");
779784
writeln!(writer, "{buf}")?;
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
incompatible: schema types changed
2+
at: MulticastGroupIdentity.id
3+
used by: POST
4+
/v1/instances/{instance}/network-interfaces/{network_interface}/multicast
5+
(instance_network_multicast_attach)
6+
 └─ MulticastGroupMember.identity
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
incompatible: schema types changed
2+
at: MulticastGroupIdentity.id
3+
used by: POST
4+
/v1/instances/{instance}/network-interfaces/{network_interface}/multicast
5+
(instance_network_multicast_attach)
6+
└─ MulticastGroupMember.identity

crates/integration-tests/tests/output/integration/blessed_version_broken.txt

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@
1010
with the blessed document (from upstream)
1111

1212
backward-incompatible: A new, required parameter 'verbose' was added [#1]
13-
at: GET /health/detailed (detailed_health_check) (.parameters.0)
13+
at: GET /health/detailed (detailed_health_check)
14+
(.parameters.0)
1415
--- a/blessed
1516
+++ b/generated
1617
@@ -1,11 +1,22 @@
@@ -41,8 +42,10 @@
4142
error: OpenAPI document generated from the current code is not compatible
4243
with the blessed document (from upstream)
4344

44-
backward-incompatible: A new, required parameter 'verbose' was added (see #1)
45-
at: GET /health/detailed (detailed_health_check) (.parameters.0)
45+
backward-incompatible: A new, required parameter 'verbose' was added (see
46+
#1)
47+
at: GET /health/detailed (detailed_health_check)
48+
(.parameters.0)
4649

4750
forward-incompatible: The operation get_system_info was added
4851
at: GET /system/info (get_system_info)

0 commit comments

Comments
 (0)