fix(view): underline and open a badge the row cut

src/view/draw/bead.rs

@@ -53,10 +53,10 @@ pub(super) fn bead_line(row: &Row, prefix: &str, id_width: usize) -> Fitted {
     let mut shorter = Vec::new();
     for badge in &row.badges {
         title.push(Span::raw(" ".repeat(GAP)));
-        if let Some(to) = &badge.link {
+        if let Some(to) = opens_at(badge) {
             links.push(Link {
                 at: title.len(),
-                to: to.clone(),
+                to: to.to_string(),
             });
         }
         if let Some(said) = badge
@@ -84,28 +84,37 @@ pub(super) fn bead_line(row: &Row, prefix: &str, id_width: usize) -> Fitted {
     fitted.toned(tone(row))
 }
 
-/// The underline is the whole of what a reader can *see* about a link: its
-/// destination is nowhere in the row's text at any width. What the terminal
-/// acts on is the hyperlink `Fitted` writes round the badge.
+/// Where the badge takes a reader, where the emitter will write a sequence
+/// saying so.
 ///
-/// So the underline is drawn on the same answer the emitter gives, rather
-/// than on the config having named a link: one the emitter refuses would draw
-/// a badge that invites a click it cannot honour.
+/// Both the underline and the hyperlink are asked of this one answer. The
+/// underline is the whole of what a reader can *see* about a link, its
+/// destination being nowhere in the row's text at any width, so a badge
+/// marked as a link that does not open is a lie and one that opens unmarked
+/// is never found.
 ///
-/// The two knobs compose because `palette::LINK` is an underline carrying no
-/// colour of its own. A badge that names neither is left with a style of
-/// nothing, which is what lets the row's own tone reach it the way it reaches
-/// the title beside it.
+/// Asked of the badge's own text rather than of the words the row goes on to
+/// draw, so the answer does not turn on how wide the row is. A badge is a
+/// link or it is not.
+fn opens_at(badge: &Badged) -> Option<&str> {
+    badge.link.as_deref().filter(|to| openable(&badge.text, to))
+}
+
+/// A badge's colour and its underline compose, because `palette::LINK` is an
+/// underline carrying no colour of its own. A badge that names neither is
+/// left with a style of nothing, which is what lets the row's own tone reach
+/// it the way it reaches the title beside it.
 fn badge_style(badge: &Badged, status: &Status) -> Style {
     let coloured = match badge.colour {
         Some(Colour::Status) => status_style(status),
         Some(Colour::Slot(slot)) => palette::slot(slot),
         Some(Colour::Absolute(colour)) => palette::absolute(colour),
         None => Style::new(),
     };
-    match &badge.link {
-        Some(to) if openable(&badge.text, to) => coloured.patch(palette::LINK),
-        _ => coloured,
+    if opens_at(badge).is_some() {
+        coloured.patch(palette::LINK)
+    } else {
+        coloured
     }
 }
 
@@ -753,20 +762,20 @@ mod tests {
         assert_eq!(drawn, said(unlinked));
     }
 
-    /// A badge the row cut has no link behind it — `surviving` keeps only the
-    /// spans the title block kept whole — so it is not drawn as one either.
+    /// A badge the row cut still points where it always did, so it is still
+    /// drawn as a link. The words are clipped; the destination is not.
     ///
     /// Read as a pair on one badge rather than as a reading at the narrow
     /// width alone. A badge that never carried a link is not underlined at
     /// any width, so the narrow reading on its own passes whether the
-    /// underline was taken off or was never there.
+    /// underline survived the cut or was never there.
     ///
     /// The badge that never carried one is read at both widths, as a whole
-    /// style rather than for its underline. The underline is all the cut
-    /// takes, and a cut that reached the badge's colour would move a badge no
-    /// link was ever drawn on.
+    /// style rather than for its underline. The cut moves nothing about a
+    /// badge's style, and a cut that reached the badge's colour would move a
+    /// badge no link was ever drawn on.
     #[test]
-    fn a_badge_the_row_cut_is_drawn_without_the_underline_it_lost_the_link_for() {
+    fn a_badge_the_row_cut_is_still_drawn_as_the_link_it_still_is() {
         let badge_at = |width: u16, to: Option<&str>| {
             let mut badged = node("smt-4kd3p.20", "a bead", Status::Blocked);
             badged.badges = vec![Badged {
@@ -788,10 +797,10 @@ mod tests {
             "the badge the row kept whole is not underlined"
         );
         assert!(
-            !badge_at(EXACTLY_THE_ROW - 1, somewhere)
+            badge_at(EXACTLY_THE_ROW - 1, somewhere)
                 .add_modifier
                 .contains(Modifier::UNDERLINED),
-            "the badge the row cut invites a click nothing is there to honour"
+            "the cut took the underline off a badge that still opens"
         );
         assert_eq!(
             badge_at(EXACTLY_THE_ROW - 1, None),
@@ -800,6 +809,78 @@ mod tests {
         );
     }
 
+    /// The underline is only a promise; what the reader acts on is the
+    /// hyperlink. A cut badge that kept the one without the other would
+    /// invite the click and drop it.
+    ///
+    /// Read at the head the row kept rather than at the badge's whole words,
+    /// because the sequence has to close inside the columns the row still
+    /// has.
+    #[test]
+    fn a_badge_the_row_cut_opens_where_a_badge_it_kept_whole_opens() {
+        let somewhere = "https://forge.invalid/orbital/atlas/pull/12";
+        let mut badged = node("smt-4kd3p.20", "a bead", Status::Blocked);
+        badged.badges = vec![Badged {
+            key: "delivery_pr".into(),
+            text: "⇢ #12".into(),
+            link: Some(somewhere.into()),
+            short: None,
+            colour: None,
+        }];
+
+        let said = symbols(bead_line(&row(&badged), BRANCH, 4), EXACTLY_THE_ROW - 1);
+
+        assert!(
+            said.contains(
+                &hyperlink("⇢ #", somewhere).expect("this vocabulary holds no control character")
+            ),
+            "the badge the row cut was not made a link: {said:?}"
+        );
+    }
+
+    /// A badge whose words the emitter refuses is not opened at any width,
+    /// including the widths that cut those words back to a head it would
+    /// accept. The row would otherwise open a link it drew no underline on,
+    /// which is the opposite mistake to the one the cut used to make and just
+    /// as much of a lie.
+    ///
+    /// Swept over every width rather than read at the one that cuts, because
+    /// which width that is falls out of a control character's own zero
+    /// columns. The clean badge is swept alongside it, so a sweep that opened
+    /// nothing anywhere cannot pass as a sweep that refused.
+    ///
+    /// Read without the note the refused badge earns the row, which would
+    /// otherwise take the columns the sweep is spending on the badge. What
+    /// the note says is `row::cells`' to say and is read there.
+    #[test]
+    fn a_badge_the_emitter_refuses_is_not_opened_at_the_widths_that_cut_it() {
+        let somewhere = "https://forge.invalid/orbital/atlas/pull/12";
+        let held = "\u{1b}]0;owned\u{7}";
+        let opened_at = |text: String, width: u16| {
+            let mut badged = node("smt-4kd3p.20", "a bead", Status::Blocked);
+            badged.badges = vec![Badged {
+                key: "delivery_pr".into(),
+                text,
+                link: Some(somewhere.into()),
+                short: None,
+                colour: None,
+            }];
+            let mut unremarked = row(&badged);
+            unremarked.notes = Vec::new();
+            symbols(bead_line(&unremarked, BRANCH, 4), width).contains(somewhere)
+        };
+        let every_width = || 1..=60;
+
+        assert!(
+            every_width().any(|width| opened_at("⇢ #12".into(), width)),
+            "the sweep opened no link at any width, so it refuses nothing"
+        );
+        assert!(
+            !every_width().any(|width| opened_at(format!("⇢ #12{held}"), width)),
+            "a badge the emitter refuses whole was opened by a cut"
+        );
+    }
+
     /// The badge that names a URL is the one the terminal is told about, and
     /// it is told round the badge's own words — so the reader clicks the badge
     /// rather than retyping what it stands for.

src/view/fitted.rs

@@ -141,8 +141,9 @@ impl Fitted {
 
     /// Which spans of the title block are links, and where each one points.
     ///
-    /// A link the row had to cut is dropped: an opening sequence with nothing
-    /// to close it makes every cell after it on the terminal part of the link.
+    /// A link the row cut is told round the head it kept, because the cut
+    /// takes columns off what the link says and nothing off where it goes. A
+    /// link the row dropped altogether has no words left to be told round.
     #[must_use]
     pub(crate) fn linking(mut self, links: Vec<Link>) -> Self {
         self.links = links;
@@ -325,12 +326,13 @@ impl Kept {
     }
 }
 
-/// The links whose span the title block kept whole, each at the column it
-/// starts on. A link whose span was cut or dropped is not among them.
-fn surviving(links: &[Link], title: &[Span<'static>], whole: usize, starts: usize) -> Vec<Kept> {
+/// The links whose span the title block drew, each at the column it starts
+/// on and saying what that span says now. A link whose span was dropped is
+/// not among them; one whose span was cut is, round the head it kept.
+fn surviving(links: &[Link], title: &[Span<'static>], drawn: usize, starts: usize) -> Vec<Kept> {
     links
         .iter()
-        .filter(|link| link.at < whole)
+        .filter(|link| link.at < drawn)
         .map(|link| Kept {
             at: starts + columns(&title[..link.at]),
             width: title[link.at].width(),
@@ -386,10 +388,10 @@ pub(crate) fn columns(spans: &[Span<'static>]) -> usize {
 /// `spans` fitted into `limit` columns: cut with the cut marked, or, for a
 /// block that says nothing in part, given up whole.
 ///
-/// Alongside the spans, how many of them the block kept whole — the leading
-/// run that says everything it was written to say. Anything after that run was
-/// cut short or dropped, and what a caller holds against a span of it no
-/// longer holds.
+/// Alongside the spans, how many of them the block drew — the leading run
+/// still standing as itself, the last of them possibly cut short. Anything
+/// after that run is nowhere on the row, and what a caller holds against a
+/// span of it has nothing left to hold it against.
 fn fit(spans: Vec<Span<'static>>, limit: usize, or_nothing: bool) -> (Vec<Span<'static>>, usize) {
     if or_nothing && columns(&spans) > limit {
         return (Vec::new(), 0);
@@ -402,8 +404,8 @@ fn fit(spans: Vec<Span<'static>>, limit: usize, or_nothing: bool) -> (Vec<Span<'
 ///
 /// Each short form is dressed in the style of the span it stands in for, so a
 /// span the row shortened is the same span saying less. It is swapped rather
-/// than cut, so `fit` counts it among the spans the block kept whole and
-/// `surviving` keeps its link.
+/// than cut, so it says a whole reference where a cut would have left a head
+/// that names none.
 ///
 /// The widest saving goes first, so that as few spans shorten as will make the
 /// run fit: a span shortened where a wider neighbour would have done is columns
@@ -435,51 +437,41 @@ fn shortened(
 }
 
 /// `spans`, cut down to `limit` columns with the cut marked, and how many of
-/// them survived whole.
+/// them reached the row, the last of those possibly cut short.
+///
+/// A span cut short keeps its style, so it is the same span saying less. The
+/// cut is told which columns it has and nothing about what it is cutting, so
+/// a style it moved would be moved for a reason it cannot see.
 fn cut_to(spans: Vec<Span<'static>>, limit: usize) -> (Vec<Span<'static>>, usize) {
     if columns(&spans) <= limit {
-        let whole = spans.len();
-        return (spans, whole);
+        let drawn = spans.len();
+        return (spans, drawn);
     }
     if limit == 0 {
         return (Vec::new(), 0);
     }
 
     let room = limit - columns(&[Span::raw(CUT.to_string())]);
     let mut kept: Vec<Span<'static>> = Vec::new();
-    let mut whole = 0;
+    let mut drawn = 0;
     let mut used = 0;
     for span in spans {
         let width = span.width();
         if used + width <= room {
             used += width;
-            whole += 1;
+            drawn += 1;
             kept.push(span);
             continue;
         }
         let head = head_of(&span.content, room - used);
         if !head.is_empty() {
-            kept.push(Span::styled(head, unlinked(span.style)));
+            drawn += 1;
+            kept.push(Span::styled(head, span.style));
         }
         break;
     }
     kept.push(Span::raw(CUT.to_string()));
-    (kept, whole)
-}
-
-/// `style` with the underline that stands for a link taken back off.
-///
-/// The other half of the rule `surviving` keeps: a span the row cut is not
-/// among the links the terminal is told about, so nothing is there to follow.
-/// Left underlined it would invite a click that cannot be honoured. Said as
-/// what `palette::LINK` adds rather than as the modifier itself, so the two
-/// cannot drift apart.
-///
-/// Asked of every span the row cuts rather than only of the ones a link
-/// names, because `cut_to` is told which columns it has and nothing about
-/// what it is cutting. A span that never carried the underline is unmoved.
-fn unlinked(style: Style) -> Style {
-    style.remove_modifier(palette::LINK.add_modifier)
+    (kept, drawn)
 }
 
 /// As much of `text` as fits in `limit` columns, never splitting a glyph.
@@ -977,16 +969,45 @@ mod tests {
         }
     }
 
-    /// A link cut for width loses the link rather than its closing sequence.
-    /// An opening sequence with nothing to close it makes every cell after it
-    /// on the terminal part of the link.
+    /// A link cut for width is opened round the head the row kept. The whole
+    /// sequence lives in the cell the link starts on, so the columns the cut
+    /// took are not columns the sequence needed.
+    ///
+    /// The width the cell reports is the head's rather than the badge's, so
+    /// the columns the diff skips behind it are the columns the link holds.
+    /// One column too many and a redraw leaves whatever stood in the next one.
+    #[test]
+    fn a_link_cut_for_width_is_opened_round_the_head_it_kept() {
+        let buf = rendered(a_linked_row(), 20);
+        let said = symbols(&buf);
+
+        assert!(
+            said.contains(CUT),
+            "the row was not cut at all, so it says nothing about a cut \
+             link: {said:?}"
+        );
+        assert!(
+            said.contains(
+                &hyperlink("⇢ #1", SOMEWHERE).expect("this vocabulary holds no control character")
+            ),
+            "a link the row cut was dropped: {said:?}"
+        );
+        assert_eq!(
+            buf[(opened_at(&buf), 0)].diff_option,
+            CellDiffOption::ForcedWidth(NonZeroU16::new(4).expect("⇢ #1 is four columns"))
+        );
+    }
+
+    /// Narrower still, and the cut leaves the link's span no columns at all.
+    /// There is nothing on the row for a sequence to be told round, and a
+    /// sequence round nothing would open a link over whatever came after it.
     #[test]
-    fn a_link_cut_for_width_is_not_opened_at_all() {
-        let said = symbols(&rendered(a_linked_row(), 20));
+    fn a_link_the_cut_left_no_room_at_all_is_not_opened() {
+        let said = symbols(&rendered(a_linked_row(), 16));
 
         assert!(
             !said.contains(OSC_8),
-            "a cut link opened a hyperlink: {said:?}"
+            "a link the row cut to nothing opened a hyperlink: {said:?}"
         );
     }