diff --git a/CHANGELOG.md b/CHANGELOG.md index ec8a622..7614d24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 example (which only dumped plaintext, not HTML) with a more useful `spv-lower-print` one ### Fixed 🩹 +- [PR#37](https://github.com/EmbarkStudios/spirt/pull/37) fixed pretty-printing layout + accuracy regarding line widths (by tracking `font-size`-aware "fractional columns"), + and raised the maximum line width back up to `120` columns - [PR#27](https://github.com/EmbarkStudios/spirt/pull/27) fixed some pretty-printing issues in the initial `Attr::Diagnostics` implementation (`BUG` paths and `/* ... */` indentation) diff --git a/src/print/mod.rs b/src/print/mod.rs index 37961b1..e9819bc 100644 --- a/src/print/mod.rs +++ b/src/print/mod.rs @@ -490,7 +490,7 @@ impl Visit for AllCxInterned { } // FIXME(eddyb) make max line width configurable. -const MAX_LINE_WIDTH: usize = 100; +const MAX_LINE_WIDTH: usize = 120; impl Plan<'_> { #[allow(rustdoc::private_intra_doc_links)] diff --git a/src/print/multiversion.rs b/src/print/multiversion.rs index 982c83a..a0e6b44 100644 --- a/src/print/multiversion.rs +++ b/src/print/multiversion.rs @@ -121,7 +121,19 @@ impl Versions { } SCOPE>tbody>tr>td { vertical-align: top; + } + SCOPE>tbody>tr>td>pre { max-width: MAX_LINE_WIDTHch; + + overflow-x: auto; + + /* HACK(eddyb) this shouldn't be needed but `visible` will turn into + `auto` because `overflow-x` is `auto` (for cursed browser reasons), + and some table cells are always assumed to need vertical scroll, + e.g. because they have `` elements on the last line (which don't + contribute to the surrounding bounding box, due to `line-height: 0`) + */ + overflow-y: hidden; } " diff --git a/src/print/pretty.rs b/src/print/pretty.rs index 349fa3a..b40cc02 100644 --- a/src/print/pretty.rs +++ b/src/print/pretty.rs @@ -96,6 +96,16 @@ impl Styles { pub fn apply(self, text: impl Into>) -> Node { Node::StyledText(Box::new((self, text.into()))) } + + // HACK(eddyb) this allows us to control ``/`` `font-size` exactly, + // and use the same information for both layout and the CSS we emit. + fn effective_size(&self) -> Option { + self.size.or(if self.subscript || self.superscript { + Some(-2) + } else { + None + }) + } } /// Color palettes built-in for convenience (colors are RGB, as `[u8; 3]`). @@ -149,6 +159,11 @@ impl Fragment { /// Perform layout on the [`Fragment`], limiting lines to `max_line_width` /// columns where possible. pub fn layout_with_max_line_width(mut self, max_line_width: usize) -> FragmentPostLayout { + // FIXME(eddyb) maybe make this a method on `Columns`? + let max_line_width = Columns { + char_width_tenths: max_line_width.try_into().unwrap_or(u16::MAX) * 10, + }; + self.approx_layout(MaxWidths { inline: max_line_width, block: max_line_width, @@ -321,7 +336,7 @@ impl<'a> FromInternalIterator> for HtmlSnippet { let mut special_tags = [ ("a", styles.anchor.is_some()), ("sub", styles.subscript), - ("super", styles.superscript), + ("sup", styles.superscript), ] .into_iter() .filter(|&(_, cond)| cond) @@ -352,9 +367,9 @@ impl<'a> FromInternalIterator> for HtmlSnippet { color, color_opacity, thickness, - size, - subscript: _, - superscript: _, + size: _, + subscript, + superscript, desaturate_and_dim_for_unchanged_multiversion_line, } = *styles; @@ -377,8 +392,12 @@ impl<'a> FromInternalIterator> for HtmlSnippet { write!(css_style, "font-weight:{};", 500 + (thickness as i32) * 100) .unwrap(); } - if let Some(size) = size { + if let Some(size) = styles.effective_size() { write!(css_style, "font-size:{}em;", 1.0 + (size as f64) * 0.1).unwrap(); + if !(subscript || superscript) { + // HACK(eddyb) without this, small text is placed too low. + write!(css_style, "vertical-align:middle;").unwrap(); + } } if desaturate_and_dim_for_unchanged_multiversion_line { write!(css_style, "filter:saturate(0.3)opacity(0.5);").unwrap(); @@ -419,6 +438,62 @@ impl<'a> FromInternalIterator> for HtmlSnippet { // Rendering implementation details (including approximate layout). +/// Fractional number of columns, used here to account for `Node::StyledText` +/// being used to intentionally reduce the size of many "helper" pieces of text, +/// at least for the HTML output (while this may lead to a less consistently +/// formatted plaintext output, making good use of width is far more important +/// for the HTML output, especially when used with `multiversion` tables). +#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] +struct Columns { + /// As our `font-size` control granularity is in multiples of `0.1em`, + /// the overall width of a line should end up a multiple of `0.1ch`, + /// i.e. we're counting tenths of a column's width at the default font size. + char_width_tenths: u16, +} + +impl Columns { + const ZERO: Self = Self { + char_width_tenths: 0, + }; + + fn text_width(text: &str) -> Self { + Self::maybe_styled_text_width(text, None) + } + + fn maybe_styled_text_width(text: &str, style: Option<&Styles>) -> Self { + assert!(!text.contains('\n')); + + let font_size = + u16::try_from(10 + style.and_then(|style| style.effective_size()).unwrap_or(0)) + .unwrap_or(0); + + // FIXME(eddyb) use `unicode-width` crate for accurate column count. + Self { + char_width_tenths: text + .len() + .try_into() + .unwrap_or(u16::MAX) + .saturating_mul(font_size), + } + } + + fn saturating_add(self, other: Self) -> Self { + Self { + char_width_tenths: self + .char_width_tenths + .saturating_add(other.char_width_tenths), + } + } + + fn saturating_sub(self, other: Self) -> Self { + Self { + char_width_tenths: self + .char_width_tenths + .saturating_sub(other.char_width_tenths), + } + } +} + /// The approximate shape of a [`Node`], regarding its 2D placement. #[derive(Copy, Clone)] enum ApproxLayout { @@ -426,23 +501,42 @@ enum ApproxLayout { /// /// `worst_width` can exceed the `inline` field of [`MaxWidths`], in which /// case the choice of inline vs block is instead made by a surrounding node. - Inline { worst_width: usize }, + Inline { + worst_width: Columns, + + /// How much of `worst_width` comes from `Node::IfBlockLayout` - that is, + /// `worst_width` still includes `Node::IfBlockLayout`, so conservative + /// decisions will still be made, but `excess_width_from_only_if_block` + /// can be used to reduce `worst_width` when block layout is no longer + /// a possibility (i.e. by the enclosing `Node::InlineOrIndentedBlock`). + excess_width_from_only_if_block: Columns, + }, /// Needs to occupy multiple lines, but may also have the equivalent of /// an `Inline` before (`pre_`) and after (`post_`) the multi-line block. // // FIXME(eddyb) maybe turn `ApproxLayout` into a `struct` instead? BlockOrMixed { - pre_worst_width: usize, - post_worst_width: usize, + pre_worst_width: Columns, + post_worst_width: Columns, }, } impl ApproxLayout { fn append(self, other: Self) -> Self { match (self, other) { - (Self::Inline { worst_width: a }, Self::Inline { worst_width: b }) => Self::Inline { + ( + Self::Inline { + worst_width: a, + excess_width_from_only_if_block: a_excess_foib, + }, + Self::Inline { + worst_width: b, + excess_width_from_only_if_block: b_excess_foib, + }, + ) => Self::Inline { worst_width: a.saturating_add(b), + excess_width_from_only_if_block: a_excess_foib.saturating_add(b_excess_foib), }, ( Self::BlockOrMixed { @@ -462,13 +556,17 @@ impl ApproxLayout { }, Self::Inline { worst_width: post_b, + excess_width_from_only_if_block: _, }, ) => Self::BlockOrMixed { pre_worst_width, post_worst_width: post_a.saturating_add(post_b), }, ( - Self::Inline { worst_width: pre_a }, + Self::Inline { + worst_width: pre_a, + excess_width_from_only_if_block: _, + }, Self::BlockOrMixed { pre_worst_width: pre_b, post_worst_width, @@ -489,8 +587,8 @@ impl ApproxLayout { /// cases, and those choices will be made inside-out based on actual widths. #[derive(Copy, Clone)] struct MaxWidths { - inline: usize, - block: usize, + inline: Columns, + block: Columns, } // FIXME(eddyb) make this configurable. @@ -504,40 +602,41 @@ impl Node { fn approx_rigid_layout(&self) -> ApproxLayout { // HACK(eddyb) workaround for the `Self::StyledText` arm not being able // to destructure through the `Box<(_, Cow)>`. - let text_approx_rigid_layout = |text: &str| { + let text_approx_rigid_layout = |text: &str, style| { if let Some((pre, non_pre)) = text.split_once('\n') { let (_, post) = non_pre.rsplit_once('\n').unwrap_or(("", non_pre)); - // FIXME(eddyb) use `unicode-width` crate for accurate column count. - let pre_width = pre.len(); - let post_width = post.len(); - ApproxLayout::BlockOrMixed { - pre_worst_width: pre_width, - post_worst_width: post_width, + pre_worst_width: Columns::maybe_styled_text_width(pre, style), + post_worst_width: Columns::maybe_styled_text_width(post, style), } } else { - // FIXME(eddyb) use `unicode-width` crate for accurate column count. - let width = text.len(); - - ApproxLayout::Inline { worst_width: width } + ApproxLayout::Inline { + worst_width: Columns::maybe_styled_text_width(text, style), + excess_width_from_only_if_block: Columns::ZERO, + } } }; #[allow(clippy::match_same_arms)] match self { - Self::Text(text) => text_approx_rigid_layout(text), - Self::StyledText(styles_and_text) => text_approx_rigid_layout(&styles_and_text.1), + Self::Text(text) => text_approx_rigid_layout(text, None), + Self::StyledText(styles_and_text) => { + text_approx_rigid_layout(&styles_and_text.1, Some(&styles_and_text.0)) + } Self::IndentedBlock(_) => ApproxLayout::BlockOrMixed { - pre_worst_width: 0, - post_worst_width: 0, + pre_worst_width: Columns::ZERO, + post_worst_width: Columns::ZERO, }, - Self::BreakingOnlySpace => ApproxLayout::Inline { worst_width: 1 }, + Self::BreakingOnlySpace => ApproxLayout::Inline { + worst_width: Columns::text_width(" "), + excess_width_from_only_if_block: Columns::ZERO, + }, Self::ForceLineSeparation => ApproxLayout::BlockOrMixed { - pre_worst_width: 0, - post_worst_width: 0, + pre_worst_width: Columns::ZERO, + post_worst_width: Columns::ZERO, }, &Self::IfBlockLayout(text) => { // Keep the inline `worst_width`, just in case this node is @@ -546,14 +645,23 @@ impl Node { // comma added by `join_comma_sep`. let text_layout = Self::Text(text.into()).approx_rigid_layout(); let worst_width = match text_layout { - ApproxLayout::Inline { worst_width } => worst_width, - ApproxLayout::BlockOrMixed { .. } => 0, + ApproxLayout::Inline { + worst_width, + excess_width_from_only_if_block: _, + } => worst_width, + ApproxLayout::BlockOrMixed { .. } => Columns::ZERO, }; - ApproxLayout::Inline { worst_width } + ApproxLayout::Inline { + worst_width, + excess_width_from_only_if_block: worst_width, + } } // Layout computed only in `approx_flex_layout`. - Self::InlineOrIndentedBlock(_) => ApproxLayout::Inline { worst_width: 0 }, + Self::InlineOrIndentedBlock(_) => ApproxLayout::Inline { + worst_width: Columns::ZERO, + excess_width_from_only_if_block: Columns::ZERO, + }, } } @@ -566,7 +674,8 @@ impl Node { match self { Self::IndentedBlock(fragments) => { // Apply one more level of indentation to the block layout. - let indented_block_max_width = max_widths.block.saturating_sub(INDENT.len()); + let indented_block_max_width = + max_widths.block.saturating_sub(Columns::text_width(INDENT)); // Recurse on `fragments`, so they can compute their own layouts. for fragment in &mut fragments[..] { @@ -577,14 +686,15 @@ impl Node { } ApproxLayout::BlockOrMixed { - pre_worst_width: 0, - post_worst_width: 0, + pre_worst_width: Columns::ZERO, + post_worst_width: Columns::ZERO, } } Self::InlineOrIndentedBlock(fragments) => { // Apply one more level of indentation to the block layout. - let indented_block_max_width = max_widths.block.saturating_sub(INDENT.len()); + let indented_block_max_width = + max_widths.block.saturating_sub(Columns::text_width(INDENT)); // Maximize the inline width available to `fragments`, usually // increasing it to the maximum allowed by the block layout. @@ -596,7 +706,10 @@ impl Node { block: indented_block_max_width, }; - let mut layout = ApproxLayout::Inline { worst_width: 0 }; + let mut layout = ApproxLayout::Inline { + worst_width: Columns::ZERO, + excess_width_from_only_if_block: Columns::ZERO, + }; for fragment in &mut fragments[..] { // Offer the same `inner_max_widths` to each `fragment`. // Worst case, they all remain inline and block layout is @@ -605,18 +718,33 @@ impl Node { layout = layout.append(fragment.approx_layout(inner_max_widths)); } - layout = match layout { - ApproxLayout::Inline { worst_width } if worst_width <= max_widths.inline => { - layout - } + // *If* we pick the inline layout, it will not end up using *any* + // `Node::OnlyIfBlock`s, so `excess_width_from_only_if_block` can + // be safely subtracted from the "candidate" inline `worst_width`. + let candidate_inline_worst_width = match layout { + ApproxLayout::Inline { + worst_width, + excess_width_from_only_if_block, + } => Some(worst_width.saturating_sub(excess_width_from_only_if_block)), + + ApproxLayout::BlockOrMixed { .. } => None, + }; + let inline_layout = candidate_inline_worst_width + .filter(|&worst_width| worst_width <= max_widths.inline) + .map(|worst_width| ApproxLayout::Inline { + worst_width, + excess_width_from_only_if_block: Columns::ZERO, + }); + + layout = inline_layout.unwrap_or( // Even if `layout` is already `ApproxLayout::BlockOrMixed`, // always reset it to a plain block, with no pre/post widths. - _ => ApproxLayout::BlockOrMixed { - pre_worst_width: 0, - post_worst_width: 0, + ApproxLayout::BlockOrMixed { + pre_worst_width: Columns::ZERO, + post_worst_width: Columns::ZERO, }, - }; + ); match layout { ApproxLayout::Inline { .. } => { @@ -636,7 +764,10 @@ impl Node { | Self::StyledText(_) | Self::BreakingOnlySpace | Self::ForceLineSeparation - | Self::IfBlockLayout(_) => ApproxLayout::Inline { worst_width: 0 }, + | Self::IfBlockLayout(_) => ApproxLayout::Inline { + worst_width: Columns::ZERO, + excess_width_from_only_if_block: Columns::ZERO, + }, } } } @@ -645,13 +776,17 @@ impl Fragment { /// Determine the [`ApproxLayout`] of this [`Fragment`], potentially making /// adjustments in order to fit within `max_widths`. fn approx_layout(&mut self, max_widths: MaxWidths) -> ApproxLayout { - let mut layout = ApproxLayout::Inline { worst_width: 0 }; + let mut layout = ApproxLayout::Inline { + worst_width: Columns::ZERO, + excess_width_from_only_if_block: Columns::ZERO, + }; let child_max_widths = |layout| MaxWidths { inline: match layout { - ApproxLayout::Inline { worst_width } => { - max_widths.inline.saturating_sub(worst_width) - } + ApproxLayout::Inline { + worst_width, + excess_width_from_only_if_block: _, + } => max_widths.inline.saturating_sub(worst_width), ApproxLayout::BlockOrMixed { post_worst_width, .. } => max_widths.block.saturating_sub(post_worst_width), @@ -676,6 +811,7 @@ impl Fragment { // process "recent" flexible nodes in between the halves. layout = layout.append(ApproxLayout::Inline { worst_width: pre_worst_width, + excess_width_from_only_if_block: Columns::ZERO, }); // FIXME(eddyb) what happens if the same node has both // rigid and flexible `ApproxLayout`s? @@ -686,7 +822,7 @@ impl Fragment { next_flex_idx += 1; } layout = layout.append(ApproxLayout::BlockOrMixed { - pre_worst_width: 0, + pre_worst_width: Columns::ZERO, post_worst_width, }); }