a73x

8213031c

Let every surface print the same id

a73x   2026-08-11 10:12

Commit message
Let every surface print the same id

The web UI and the dashboard hard-coded eight characters while the CLI
went through `abbrev`. Past ~32k objects of a kind they would have
disagreed outright, but the sharper problem was there all along: both
computed their short ids from the rows they were about to render, and
the rows are a filtered subset. An id unique among the open issues on
one page can name two issues in the repository.

That is why this is not a find-and-replace. The width has to come from a
query the view does not otherwise need — every id of the kind, archived
included — so a filtered page can widen because of an object it is not
showing. Both surfaces now run that query alongside the one that
produces their rows, and `patch checkout` joins them: the branch it
names after a patch is the one displayed id guaranteed to outlive the
command, and two patches sharing eight characters would have wanted the
same branch, with the suffix loop resolving it to `collab/<id>-1` —
which reads as a second checkout of one patch, not a checkout of another.

Commit oids keep git's own short form. They are not collab ids, all
three surfaces already agree on them, and `core.abbrev` is the setting
that governs them. Comment ids likewise: a comment is named only within
the issue or patch that holds it, and `resolve_comment` searches that
object's own comments, so its population is a handful of siblings rather
than every object of a kind. Feeding one to the issue abbreviator would
pick a width from a set it is not a member of.

Tests plant an archived twin sharing a real id's first eight characters.
Eight is the floor, so no repository small enough to build in a test
collides there by chance, and an archived object is in the id set the
policy is computed over and in none of the views that list objects —
exactly the case a width taken from the visible rows gets wrong, and
silently, because the page still looks right. Both surfaces are also
pinned as read-only: a page view and a dashboard launch must leave every
collab ref where it was.

Fixes: 10bb2d84

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
src/patch.rs
Old New
@@ -1374,7 +1374,14 @@ pub fn checkout(repo: &Repository, id_prefix: &str) -> Result<(), crate::error::
1374 .find_commit(commit_oid) 1374 .find_commit(commit_oid)
1375 .map_err(|e| Error::Cmd(format!("commit {} not found: {}", &latest_rev.commit, e)))?; 1375 .map_err(|e| Error::Cmd(format!("commit {} not found: {}", &latest_rev.commit, e)))?;
1376 1376
1377 let short_id = &id[..std::cmp::min(8, id.len())]; 1377 // The branch name is a displayed id — the one that outlives the command
1378 // by construction, since the branch is left behind — so it is abbreviated
1379 // by the same policy as everything else printed. A hard-coded eight would
1380 // give two patches sharing eight characters the same candidate name, and
1381 // the suffix loop below would resolve that to `collab/<id>-1`, which reads
1382 // as a second checkout of one patch rather than a checkout of another.
1383 let abbrev = crate::abbrev::for_patches(repo);
1384 let short_id = abbrev.of(&id);
1378 // Reusing a branch that already stands at the wanted commit is what keeps a 1385 // Reusing a branch that already stands at the wanted commit is what keeps a
1379 // review session from piling up `collab/<id>-1`, `-2`, `-3`. A branch of the 1386 // review session from piling up `collab/<id>-1`, `-2`, `-3`. A branch of the
1380 // same name pointing somewhere else is someone's work, so that one is left 1387 // same name pointing somewhere else is someone's work, so that one is left
src/server/http/repo/issues.rs
Old New
@@ -117,11 +117,18 @@ pub async fn issues(
117 } 117 }
118 }; 118 };
119 119
120 // Built from every issue id in the repository, not from the rows above.
121 // The two are deliberately different queries: an id that is unique among
122 // the issues this filter happens to show may name two issues in the repo,
123 // and the id printed here is a reference that leaves the page — into a
124 // `Issue:` trailer, a branch name, a script. See `abbrev`.
125 let abbrev = git_collab::abbrev::for_issues(&repo);
126
120 let issues = filtered_issues 127 let issues = filtered_issues
121 .into_iter() 128 .into_iter()
122 .map(|i| { 129 .map(|i| {
123 let id = i.id.clone(); 130 let id = i.id.clone();
124 let short_id = id[..8.min(id.len())].to_string(); 131 let short_id = abbrev.of(&id).to_string();
125 IssueListItem { 132 IssueListItem {
126 short_id, 133 short_id,
127 id, 134 id,
src/server/http/repo/overview.rs
Old New
@@ -11,6 +11,10 @@ use super::{
11 #[derive(Debug)] 11 #[derive(Debug)]
12 pub struct OverviewPatch { 12 pub struct OverviewPatch {
13 pub id: String, 13 pub id: String,
14 /// Abbreviated here rather than in the template: a slice in the template
15 /// is both a hard-coded width and a panic waiting for an id shorter than
16 /// it, inside a handler where a panic drops the connection.
17 pub short_id: String,
14 pub title: String, 18 pub title: String,
15 pub author: String, 19 pub author: String,
16 } 20 }
@@ -18,6 +22,7 @@ pub struct OverviewPatch {
18 #[derive(Debug)] 22 #[derive(Debug)]
19 pub struct OverviewIssue { 23 pub struct OverviewIssue {
20 pub id: String, 24 pub id: String,
25 pub short_id: String,
21 pub title: String, 26 pub title: String,
22 pub author: String, 27 pub author: String,
23 } 28 }
@@ -50,11 +55,19 @@ pub async fn overview(
50 let branch = head_branch_name(&repo); 55 let branch = head_branch_name(&repo);
51 let readme = readme::load_readme(&repo, &repo_name, &branch); 56 let readme = readme::load_readme(&repo, &repo_name, &branch);
52 57
58 // The overview is the most heavily filtered view there is — open objects
59 // only — so it is the one most likely to be abbreviated too narrowly if
60 // the width came from its own rows. Both abbreviators are built from every
61 // id of their kind instead.
62 let patch_abbrev = git_collab::abbrev::for_patches(&repo);
63 let issue_abbrev = git_collab::abbrev::for_issues(&repo);
64
53 let patches = git_collab::state::list_patches(&repo) 65 let patches = git_collab::state::list_patches(&repo)
54 .unwrap_or_default() 66 .unwrap_or_default()
55 .into_iter() 67 .into_iter()
56 .filter(|p| p.status == git_collab::state::PatchStatus::Open) 68 .filter(|p| p.status == git_collab::state::PatchStatus::Open)
57 .map(|p| OverviewPatch { 69 .map(|p| OverviewPatch {
70 short_id: patch_abbrev.of(&p.id).to_string(),
58 id: p.id, 71 id: p.id,
59 title: p.title, 72 title: p.title,
60 author: p.author.name, 73 author: p.author.name,
@@ -66,6 +79,7 @@ pub async fn overview(
66 .into_iter() 79 .into_iter()
67 .filter(|i| i.status == git_collab::state::IssueStatus::Open) 80 .filter(|i| i.status == git_collab::state::IssueStatus::Open)
68 .map(|i| OverviewIssue { 81 .map(|i| OverviewIssue {
82 short_id: issue_abbrev.of(&i.id).to_string(),
69 id: i.id, 83 id: i.id,
70 title: i.title, 84 title: i.title,
71 author: i.author.name, 85 author: i.author.name,
src/server/http/repo/patches.rs
Old New
@@ -177,11 +177,15 @@ pub async fn patches(
177 } 177 }
178 }; 178 };
179 179
180 // Built from every patch id in the repository, not from the rows above —
181 // see the matching comment in `issues.rs`.
182 let abbrev = git_collab::abbrev::for_patches(&repo);
183
180 let patches = filtered_patches 184 let patches = filtered_patches
181 .into_iter() 185 .into_iter()
182 .map(|p| { 186 .map(|p| {
183 let id = p.id.clone(); 187 let id = p.id.clone();
184 let short_id = id[..8.min(id.len())].to_string(); 188 let short_id = abbrev.of(&id).to_string();
185 PatchListItem { 189 PatchListItem {
186 short_id, 190 short_id,
187 id, 191 id,
src/server/http/templates/repo_overview.html
Old New
@@ -26,7 +26,7 @@
26 <tbody> 26 <tbody>
27 {% for patch in patches %} 27 {% for patch in patches %}
28 <tr> 28 <tr>
29 <td class="mono"><a href="/{{ repo_name }}/patches/{{ patch.id }}">{{ patch.id[..8] }}</a></td> 29 <td class="mono"><a href="/{{ repo_name }}/patches/{{ patch.id }}">{{ patch.short_id }}</a></td>
30 <td><a href="/{{ repo_name }}/patches/{{ patch.id }}">{{ patch.title }}</a></td> 30 <td><a href="/{{ repo_name }}/patches/{{ patch.id }}">{{ patch.title }}</a></td>
31 <td style="color: #666;">{{ patch.author }}</td> 31 <td style="color: #666;">{{ patch.author }}</td>
32 </tr> 32 </tr>
@@ -52,7 +52,7 @@
52 <tbody> 52 <tbody>
53 {% for issue in issues %} 53 {% for issue in issues %}
54 <tr> 54 <tr>
55 <td class="mono"><a href="/{{ repo_name }}/issues/{{ issue.id }}">{{ issue.id[..8] }}</a></td> 55 <td class="mono"><a href="/{{ repo_name }}/issues/{{ issue.id }}">{{ issue.short_id }}</a></td>
56 <td><a href="/{{ repo_name }}/issues/{{ issue.id }}">{{ issue.title }}</a></td> 56 <td><a href="/{{ repo_name }}/issues/{{ issue.id }}">{{ issue.title }}</a></td>
57 <td style="color: #666;">{{ issue.author }}</td> 57 <td style="color: #666;">{{ issue.author }}</td>
58 </tr> 58 </tr>
src/tui/events.rs
Old New
@@ -89,7 +89,10 @@ pub(crate) fn run_loop(
89 match issue_mod::open(repo, &title, "", None) { 89 match issue_mod::open(repo, &title, "", None) {
90 Ok(id) => { 90 Ok(id) => {
91 app.reload(repo); 91 app.reload(repo);
92 app.status_msg = Some(format!("Issue created: {:.8}", id)); 92 app.status_msg = Some(format!(
93 "Issue created: {}",
94 app.issue_abbrev.of(&id)
95 ));
93 } 96 }
94 Err(e) => { 97 Err(e) => {
95 app.status_msg = 98 app.status_msg =
@@ -106,7 +109,10 @@ pub(crate) fn run_loop(
106 match issue_mod::open(repo, &title, &body, None) { 109 match issue_mod::open(repo, &title, &body, None) {
107 Ok(id) => { 110 Ok(id) => {
108 app.reload(repo); 111 app.reload(repo);
109 app.status_msg = Some(format!("Issue created: {:.8}", id)); 112 app.status_msg = Some(format!(
113 "Issue created: {}",
114 app.issue_abbrev.of(&id)
115 ));
110 } 116 }
111 Err(e) => { 117 Err(e) => {
112 app.status_msg = 118 app.status_msg =
src/tui/mod.rs
Old New
@@ -18,8 +18,13 @@ use self::state::App;
18 pub fn run(repo: &Repository) -> Result<(), Error> { 18 pub fn run(repo: &Repository) -> Result<(), Error> {
19 let issues = app_state::list_issues(repo)?; 19 let issues = app_state::list_issues(repo)?;
20 let patches = app_state::list_patches(repo)?; 20 let patches = app_state::list_patches(repo)?;
21 // A separate query from the two above, and that is the point: those list
22 // what the dashboard can display, this counts what the ids have to be
23 // unique against.
24 let issue_abbrev = crate::abbrev::for_issues(repo);
25 let patch_abbrev = crate::abbrev::for_patches(repo);
21 26
22 let mut app = App::new(issues, patches); 27 let mut app = App::new(issues, patches, issue_abbrev, patch_abbrev);
23 28
24 terminal::enable_raw_mode()?; 29 terminal::enable_raw_mode()?;
25 stdout().execute(EnterAlternateScreen)?; 30 stdout().execute(EnterAlternateScreen)?;
@@ -37,6 +42,7 @@ pub fn run(repo: &Repository) -> Result<(), Error> {
37 mod tests { 42 mod tests {
38 use super::state::*; 43 use super::state::*;
39 use super::widgets::*; 44 use super::widgets::*;
45 use crate::abbrev::Abbrev;
40 use crate::event::{Action, Author, ReviewVerdict}; 46 use crate::event::{Action, Author, ReviewVerdict};
41 use crate::state::{IssueState, IssueStatus, PatchState, PatchStatus}; 47 use crate::state::{IssueState, IssueStatus, PatchState, PatchStatus};
42 use git2::Oid; 48 use git2::Oid;
@@ -103,7 +109,7 @@ mod tests {
103 make_patch("p2", "Dashboard patch", PatchStatus::Closed), 109 make_patch("p2", "Dashboard patch", PatchStatus::Closed),
104 make_patch("p3", "Merged feature", PatchStatus::Merged), 110 make_patch("p3", "Merged feature", PatchStatus::Merged),
105 ]; 111 ];
106 App::new(issues, patches) 112 App::new(issues, patches, Abbrev::minimal(), Abbrev::minimal())
107 } 113 }
108 114
109 // T010: visible_issues filters by search_query (case-insensitive) 115 // T010: visible_issues filters by search_query (case-insensitive)
@@ -292,7 +298,12 @@ mod tests {
292 } 298 }
293 299
294 fn make_app(issues: usize, patches: usize) -> App { 300 fn make_app(issues: usize, patches: usize) -> App {
295 App::new(make_test_issues(issues), make_test_patches(patches)) 301 App::new(
302 make_test_issues(issues),
303 make_test_patches(patches),
304 Abbrev::minimal(),
305 Abbrev::minimal(),
306 )
296 } 307 }
297 308
298 fn render_app(app: &mut App) -> Buffer { 309 fn render_app(app: &mut App) -> Buffer {
@@ -461,7 +472,7 @@ mod tests {
461 }, 472 },
462 clock: 0, 473 clock: 0,
463 }; 474 };
464 let detail = format_event_detail(&oid, &event); 475 let detail = format_event_detail(&oid, &event, &Abbrev::minimal());
465 assert!(detail.contains("aaaaaaa")); 476 assert!(detail.contains("aaaaaaa"));
466 assert!(detail.contains("Test User <test@example.com>")); 477 assert!(detail.contains("Test User <test@example.com>"));
467 assert!(detail.contains("2026-01-01T00:00:00Z")); 478 assert!(detail.contains("2026-01-01T00:00:00Z"));
@@ -481,7 +492,7 @@ mod tests {
481 }, 492 },
482 clock: 0, 493 clock: 0,
483 }; 494 };
484 let detail = format_event_detail(&oid, &event); 495 let detail = format_event_detail(&oid, &event, &Abbrev::minimal());
485 assert!(detail.contains("Issue Close")); 496 assert!(detail.contains("Issue Close"));
486 assert!(detail.contains("Reason: resolved")); 497 assert!(detail.contains("Reason: resolved"));
487 } 498 }
@@ -499,7 +510,7 @@ mod tests {
499 }, 510 },
500 clock: 0, 511 clock: 0,
501 }; 512 };
502 let detail = format_event_detail(&oid, &event); 513 let detail = format_event_detail(&oid, &event, &Abbrev::minimal());
503 assert!(detail.contains("Patch Review")); 514 assert!(detail.contains("Patch Review"));
504 assert!(detail.contains("approve")); 515 assert!(detail.contains("approve"));
505 assert!(detail.contains("Looks good!")); 516 assert!(detail.contains("Looks good!"));
@@ -514,7 +525,7 @@ mod tests {
514 action: Action::IssueReopen, 525 action: Action::IssueReopen,
515 clock: 0, 526 clock: 0,
516 }; 527 };
517 let detail = format_event_detail(&oid, &event); 528 let detail = format_event_detail(&oid, &event, &Abbrev::minimal());
518 assert!(detail.contains("1234567")); 529 assert!(detail.contains("1234567"));
519 assert!(detail.contains("Commit: 1234567\n")); 530 assert!(detail.contains("Commit: 1234567\n"));
520 } 531 }
@@ -903,6 +914,68 @@ mod tests {
903 assert_buffer_contains(&buf, "No matches for current filter."); 914 assert_buffer_contains(&buf, "No matches for current filter.");
904 } 915 }
905 916
917 // ── Id width ─────────────────────────────────────────────────────────
918 //
919 // The dashboard's lists are filtered twice over — by status, then by the
920 // search box — and its issue set never contains an archived issue at all.
921 // The width has to come from the abbreviator it was handed, which was
922 // built over every object of the kind, and not from the rows it can see.
923
924 /// An id whose only twin is off-screen must still be printed wide enough
925 /// to tell them apart. This is the failure a width derived from the
926 /// visible rows produces, and it is silent: the list looks right.
927 #[test]
928 fn test_render_widens_an_id_whose_twin_is_not_on_screen() {
929 let on_screen = format!("{:0<40}", "abcdef01aa");
930 let off_screen = format!("{:0<40}", "abcdef01bb");
931
932 let mut app = App::new(
933 vec![make_issue(
934 &on_screen,
935 "The only visible issue",
936 IssueStatus::Open,
937 )],
938 vec![],
939 Abbrev::new(vec![on_screen.clone(), off_screen]),
940 Abbrev::minimal(),
941 );
942
943 let buf = render_app(&mut app);
944 let text = buffer_to_string(&buf);
945 assert!(
946 text.contains(&on_screen[..9]),
947 "expected the id widened past the floor, got:\n{}",
948 text
949 );
950 assert!(
951 !text.contains(&format!("{} ", &on_screen[..8])),
952 "the eight-character form names two issues:\n{}",
953 text
954 );
955 }
956
957 /// A configured width reaches the list and the detail pane alike, so the
958 /// id a reader copies is the same one either way.
959 #[test]
960 fn test_render_honours_a_configured_width() {
961 let id = format!("{:0<40}", "abcdef0123456789");
962 let mut app = App::new(
963 vec![make_issue(&id, "Configured", IssueStatus::Open)],
964 vec![],
965 Abbrev::with_width(vec![id.clone()], Some(14)),
966 Abbrev::minimal(),
967 );
968
969 let buf = render_app(&mut app);
970 let text = buffer_to_string(&buf);
971 assert!(text.contains(&id[..14]), "list pane:\n{}", text);
972 assert!(
973 text.contains(&format!("Issue {}", &id[..14])),
974 "detail pane:\n{}",
975 text
976 );
977 }
978
906 #[test] 979 #[test]
907 fn test_render_footer_keys() { 980 fn test_render_footer_keys() {
908 let mut app = make_app(3, 3); 981 let mut app = make_app(3, 3);
src/tui/state.rs
Old New
@@ -2,6 +2,7 @@ use crossterm::event::{KeyCode, KeyModifiers};
2 use git2::{Oid, Repository}; 2 use git2::{Oid, Repository};
3 use ratatui::widgets::ListState; 3 use ratatui::widgets::ListState;
4 4
5 use crate::abbrev::Abbrev;
5 use crate::state::{self, IssueState, IssueStatus, PatchState, PatchStatus}; 6 use crate::state::{self, IssueState, IssueStatus, PatchState, PatchStatus};
6 7
7 #[derive(Debug, PartialEq)] 8 #[derive(Debug, PartialEq)]
@@ -70,6 +71,18 @@ pub(crate) enum InputMode {
70 pub(crate) struct App { 71 pub(crate) struct App {
71 pub(crate) issues: Vec<IssueState>, 72 pub(crate) issues: Vec<IssueState>,
72 pub(crate) patches: Vec<PatchState>, 73 pub(crate) patches: Vec<PatchState>,
74 /// How wide to print an issue id, and likewise for patches.
75 ///
76 /// Deliberately not derived from `issues` and `patches` above. Those hold
77 /// what the dashboard can show — active objects, further narrowed by the
78 /// status filter and the search box before anything reaches the screen —
79 /// while the width has to come from every object of that kind, archived
80 /// ones included. An id abbreviated against the rows on screen is unique
81 /// only among them, and an id read off this screen is typed back into a
82 /// command, a trailer or a branch name, where "unique among whatever was
83 /// on screen at the time" is not a property anything can rely on.
84 pub(crate) issue_abbrev: Abbrev,
85 pub(crate) patch_abbrev: Abbrev,
73 pub(crate) list_state: ListState, 86 pub(crate) list_state: ListState,
74 pub(crate) patch_list_state: ListState, 87 pub(crate) patch_list_state: ListState,
75 pub(crate) list_mode: ListMode, 88 pub(crate) list_mode: ListMode,
@@ -93,7 +106,12 @@ pub(crate) struct App {
93 } 106 }
94 107
95 impl App { 108 impl App {
96 pub(crate) fn new(issues: Vec<IssueState>, patches: Vec<PatchState>) -> Self { 109 pub(crate) fn new(
110 issues: Vec<IssueState>,
111 patches: Vec<PatchState>,
112 issue_abbrev: Abbrev,
113 patch_abbrev: Abbrev,
114 ) -> Self {
97 let mut list_state = ListState::default(); 115 let mut list_state = ListState::default();
98 if !issues.is_empty() { 116 if !issues.is_empty() {
99 list_state.select(Some(0)); 117 list_state.select(Some(0));
@@ -105,6 +123,8 @@ impl App {
105 Self { 123 Self {
106 issues, 124 issues,
107 patches, 125 patches,
126 issue_abbrev,
127 patch_abbrev,
108 list_state, 128 list_state,
109 patch_list_state, 129 patch_list_state,
110 list_mode: ListMode::Issues, 130 list_mode: ListMode::Issues,
@@ -444,6 +464,12 @@ impl App {
444 if let Ok(patches) = state::list_patches(repo) { 464 if let Ok(patches) = state::list_patches(repo) {
445 self.patches = patches; 465 self.patches = patches;
446 } 466 }
467 // Refreshing the lists without refreshing the widths would leave the
468 // dashboard printing ids at a width chosen for a repository that no
469 // longer exists — including right after `n` opens an issue, which is
470 // exactly when a new id collides. Both are ref and config reads.
471 self.issue_abbrev = crate::abbrev::for_issues(repo);
472 self.patch_abbrev = crate::abbrev::for_patches(repo);
447 // Clamp issue list selection 473 // Clamp issue list selection
448 let issue_len = self.visible_issues().len(); 474 let issue_len = self.visible_issues().len();
449 if let Some(sel) = self.list_state.selected() { 475 if let Some(sel) = self.list_state.selected() {
src/tui/widgets.rs
Old New
@@ -2,6 +2,7 @@ use git2::{Oid, Repository};
2 use ratatui::prelude::*; 2 use ratatui::prelude::*;
3 use ratatui::widgets::{Block, Borders, List, ListItem, Paragraph, Wrap}; 3 use ratatui::widgets::{Block, Borders, List, ListItem, Paragraph, Wrap};
4 4
5 use crate::abbrev::Abbrev;
5 use crate::event::{Action, ReviewVerdict}; 6 use crate::event::{Action, ReviewVerdict};
6 use crate::state::{IssueState, IssueStatus, PatchState, PatchStatus}; 7 use crate::state::{IssueState, IssueStatus, PatchState, PatchStatus};
7 8
@@ -66,7 +67,17 @@ pub(crate) fn action_type_label(action: &Action) -> &str {
66 } 67 }
67 } 68 }
68 69
69 pub(crate) fn format_event_detail(oid: &Oid, event: &crate::event::Event) -> String { 70 /// Render one raw event for the commit browser.
71 ///
72 /// `issue_abbrev` governs the issue ids in the payload. The event's own commit
73 /// oid, and the `target` of an edit or a delete, are git object ids naming an
74 /// event commit rather than collab ids, so they keep git's own short form —
75 /// `collab.abbrev` has nothing to say about them.
76 pub(crate) fn format_event_detail(
77 oid: &Oid,
78 event: &crate::event::Event,
79 issue_abbrev: &Abbrev,
80 ) -> String {
70 let short_oid = &oid.to_string()[..7]; 81 let short_oid = &oid.to_string()[..7];
71 let action_label = action_type_label(&event.action); 82 let action_label = action_type_label(&event.action);
72 83
@@ -147,10 +158,13 @@ pub(crate) fn format_event_detail(oid: &Oid, event: &crate::event::Event) -> Str
147 detail.push_str(&format!("\nRemoved Label: {}\n", label)); 158 detail.push_str(&format!("\nRemoved Label: {}\n", label));
148 } 159 }
149 Action::IssueRelate { relates_to } => { 160 Action::IssueRelate { relates_to } => {
150 detail.push_str(&format!("\nRelates To: {:.8}\n", relates_to)); 161 detail.push_str(&format!("\nRelates To: {}\n", issue_abbrev.of(relates_to)));
151 } 162 }
152 Action::IssueUnrelate { relates_to } => { 163 Action::IssueUnrelate { relates_to } => {
153 detail.push_str(&format!("\nRemoved Relation: {:.8}\n", relates_to)); 164 detail.push_str(&format!(
165 "\nRemoved Relation: {}\n",
166 issue_abbrev.of(relates_to)
167 ));
154 } 168 }
155 Action::IssueAssign { assignee } => { 169 Action::IssueAssign { assignee } => {
156 detail.push_str(&format!("\nAssignee: {}\n", assignee)); 170 detail.push_str(&format!("\nAssignee: {}\n", assignee));
@@ -216,7 +230,13 @@ fn render_list(frame: &mut Frame, app: &mut App, area: Rect) {
216 IssueStatus::Open => Style::default().fg(Color::Green), 230 IssueStatus::Open => Style::default().fg(Color::Green),
217 IssueStatus::Closed => Style::default().fg(Color::Red), 231 IssueStatus::Closed => Style::default().fg(Color::Red),
218 }; 232 };
219 ListItem::new(format!("{:.8} {:6} {}", i.id, status, i.title)).style(style) 233 ListItem::new(format!(
234 "{} {:6} {}",
235 app.issue_abbrev.of(&i.id),
236 status,
237 i.title
238 ))
239 .style(style)
220 }) 240 })
221 .collect(); 241 .collect();
222 242
@@ -249,7 +269,13 @@ fn render_list(frame: &mut Frame, app: &mut App, area: Rect) {
249 PatchStatus::Closed => Style::default().fg(Color::Red), 269 PatchStatus::Closed => Style::default().fg(Color::Red),
250 PatchStatus::Merged => Style::default().fg(Color::Cyan), 270 PatchStatus::Merged => Style::default().fg(Color::Cyan),
251 }; 271 };
252 ListItem::new(format!("{:.8} {:6} {}", p.id, status, p.title)).style(style) 272 ListItem::new(format!(
273 "{} {:6} {}",
274 app.patch_abbrev.of(&p.id),
275 status,
276 p.title
277 ))
278 .style(style)
253 }) 279 })
254 .collect(); 280 .collect();
255 281
@@ -331,7 +357,7 @@ fn render_detail(frame: &mut Frame, app: &mut App, area: Rect, repo: Option<&Rep
331 if app.mode == ViewMode::CommitDetail { 357 if app.mode == ViewMode::CommitDetail {
332 let content = if let Some(idx) = app.event_list_state.selected() { 358 let content = if let Some(idx) = app.event_list_state.selected() {
333 if let Some((oid, evt)) = app.event_history.get(idx) { 359 if let Some((oid, evt)) = app.event_history.get(idx) {
334 format_event_detail(oid, evt) 360 format_event_detail(oid, evt, &app.issue_abbrev)
335 } else { 361 } else {
336 "No event selected.".to_string() 362 "No event selected.".to_string()
337 } 363 }
@@ -358,7 +384,13 @@ fn render_detail(frame: &mut Frame, app: &mut App, area: Rect, repo: Option<&Rep
358 let visible = app.visible_issues(); 384 let visible = app.visible_issues();
359 let selected_idx = app.list_state.selected().unwrap_or(0); 385 let selected_idx = app.list_state.selected().unwrap_or(0);
360 let content: Text = match visible.get(selected_idx) { 386 let content: Text = match visible.get(selected_idx) {
361 Some(issue) => build_issue_detail(issue, &app.patches, repo), 387 Some(issue) => build_issue_detail(
388 issue,
389 &app.patches,
390 repo,
391 &app.issue_abbrev,
392 &app.patch_abbrev,
393 ),
362 None => Text::raw("No matches for current filter."), 394 None => Text::raw("No matches for current filter."),
363 }; 395 };
364 396
@@ -378,7 +410,7 @@ fn render_detail(frame: &mut Frame, app: &mut App, area: Rect, repo: Option<&Rep
378 let visible = app.visible_patches(); 410 let visible = app.visible_patches();
379 let selected_idx = app.patch_list_state.selected().unwrap_or(0); 411 let selected_idx = app.patch_list_state.selected().unwrap_or(0);
380 let content: Text = match visible.get(selected_idx) { 412 let content: Text = match visible.get(selected_idx) {
381 Some(patch) => build_patch_summary(patch), 413 Some(patch) => build_patch_summary(patch, &app.issue_abbrev, &app.patch_abbrev),
382 None => Text::raw("No patches for current filter."), 414 None => Text::raw("No patches for current filter."),
383 }; 415 };
384 416
@@ -401,6 +433,8 @@ fn build_issue_detail(
401 issue: &IssueState, 433 issue: &IssueState,
402 patches: &[PatchState], 434 patches: &[PatchState],
403 repo: Option<&Repository>, 435 repo: Option<&Repository>,
436 issue_abbrev: &Abbrev,
437 patch_abbrev: &Abbrev,
404 ) -> Text<'static> { 438 ) -> Text<'static> {
405 let status = issue.status.as_str(); 439 let status = issue.status.as_str();
406 440
@@ -408,7 +442,7 @@ fn build_issue_detail(
408 Line::from(vec![ 442 Line::from(vec![
409 Span::styled("Issue ", Style::default().add_modifier(Modifier::BOLD)), 443 Span::styled("Issue ", Style::default().add_modifier(Modifier::BOLD)),
410 Span::styled( 444 Span::styled(
411 format!("{:.8}", issue.id), 445 issue_abbrev.of(&issue.id).to_string(),
412 Style::default() 446 Style::default()
413 .fg(Color::Yellow) 447 .fg(Color::Yellow)
414 .add_modifier(Modifier::BOLD), 448 .add_modifier(Modifier::BOLD),
@@ -477,7 +511,10 @@ fn build_issue_detail(
477 PatchStatus::Merged => ("merged", Color::Cyan), 511 PatchStatus::Merged => ("merged", Color::Cyan),
478 }; 512 };
479 lines.push(Line::from(vec![ 513 lines.push(Line::from(vec![
480 Span::styled(format!("{:.8}", p.id), Style::default().fg(Color::Yellow)), 514 Span::styled(
515 patch_abbrev.of(&p.id).to_string(),
516 Style::default().fg(Color::Yellow),
517 ),
481 Span::raw(" "), 518 Span::raw(" "),
482 Span::styled(status.0, Style::default().fg(status.1)), 519 Span::styled(status.0, Style::default().fg(status.1)),
483 Span::raw(format!(" {}", p.title)), 520 Span::raw(format!(" {}", p.title)),
@@ -562,7 +599,11 @@ fn build_issue_detail(
562 Text::from(lines) 599 Text::from(lines)
563 } 600 }
564 601
565 fn build_patch_summary(patch: &PatchState) -> Text<'static> { 602 fn build_patch_summary(
603 patch: &PatchState,
604 issue_abbrev: &Abbrev,
605 patch_abbrev: &Abbrev,
606 ) -> Text<'static> {
566 let status_str = patch.status.as_str(); 607 let status_str = patch.status.as_str();
567 let status_color = match patch.status { 608 let status_color = match patch.status {
568 PatchStatus::Open => Color::Green, 609 PatchStatus::Open => Color::Green,
@@ -574,7 +615,7 @@ fn build_patch_summary(patch: &PatchState) -> Text<'static> {
574 Line::from(vec![ 615 Line::from(vec![
575 Span::styled("Patch ", Style::default().add_modifier(Modifier::BOLD)), 616 Span::styled("Patch ", Style::default().add_modifier(Modifier::BOLD)),
576 Span::styled( 617 Span::styled(
577 format!("{:.8}", patch.id), 618 patch_abbrev.of(&patch.id).to_string(),
578 Style::default() 619 Style::default()
579 .fg(Color::Yellow) 620 .fg(Color::Yellow)
580 .add_modifier(Modifier::BOLD), 621 .add_modifier(Modifier::BOLD),
@@ -607,7 +648,8 @@ fn build_patch_summary(patch: &PatchState) -> Text<'static> {
607 if let Some(ref fixes) = patch.fixes { 648 if let Some(ref fixes) = patch.fixes {
608 lines.push(Line::from(vec![ 649 lines.push(Line::from(vec![
609 Span::styled("Fixes: ", Style::default().fg(Color::DarkGray)), 650 Span::styled("Fixes: ", Style::default().fg(Color::DarkGray)),
610 Span::raw(format!("{:.8}", fixes)), 651 // `fixes` names an issue, so it is abbreviated against issues.
652 Span::raw(issue_abbrev.of(fixes).to_string()),
611 ])); 653 ]));
612 } 654 }
613 655
@@ -644,7 +686,7 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
644 Line::from(vec![ 686 Line::from(vec![
645 Span::styled("Patch ", Style::default().add_modifier(Modifier::BOLD)), 687 Span::styled("Patch ", Style::default().add_modifier(Modifier::BOLD)),
646 Span::styled( 688 Span::styled(
647 format!("{:.8}", patch.id), 689 app.patch_abbrev.of(&patch.id).to_string(),
648 Style::default() 690 Style::default()
649 .fg(Color::Yellow) 691 .fg(Color::Yellow)
650 .add_modifier(Modifier::BOLD), 692 .add_modifier(Modifier::BOLD),
@@ -684,7 +726,7 @@ fn build_patch_detail_text(app: &App) -> Text<'static> {
684 if let Some(ref fixes) = patch.fixes { 726 if let Some(ref fixes) = patch.fixes {
685 lines.push(Line::from(vec![ 727 lines.push(Line::from(vec![
686 Span::styled("Fixes: ", Style::default().fg(Color::DarkGray)), 728 Span::styled("Fixes: ", Style::default().fg(Color::DarkGray)),
687 Span::raw(format!("{:.8}", fixes)), 729 Span::raw(app.issue_abbrev.of(fixes).to_string()),
688 ])); 730 ]));
689 } 731 }
690 732
tests/abbrev_test.rs
Old New
@@ -4,10 +4,15 @@
4 //! the two promises the CLI makes about it: an id it prints can always be 4 //! the two promises the CLI makes about it: an id it prints can always be
5 //! typed straight back in, and a prefix that names more than one object is 5 //! typed straight back in, and a prefix that names more than one object is
6 //! refused rather than resolved. 6 //! refused rather than resolved.
7 //!
8 //! The `web ui` and `dashboard` sections at the bottom hold the same promises
9 //! to the other two surfaces, which have their own way of getting them wrong:
10 //! both render a *filtered* subset, and a width computed from what is on
11 //! screen is not the width the policy asks for.
7 12
8 mod common; 13 mod common;
9 14
10 use common::TestRepo; 15 use common::{ServerHarness, TestRepo};
11 16
12 /// Pull the leading id column out of a `list` line. 17 /// Pull the leading id column out of a `list` line.
13 fn first_column(line: &str) -> &str { 18 fn first_column(line: &str) -> &str {
@@ -357,3 +362,466 @@ fn the_id_set_covers_archived_patches() {
357 all 362 all
358 ); 363 );
359 } 364 }
365
366 // ---------------------------------------------------------------------------
367 // The other two surfaces
368 //
369 // The web UI and the dashboard both render a *filtered* subset — open issues,
370 // merged patches, whatever the current view asks for. The policy is computed
371 // over every object of a kind, so a filtered view has to be abbreviated at a
372 // width it cannot derive from its own rows. These tests are written to fail
373 // against a surface that abbreviates over what it is showing.
374 // ---------------------------------------------------------------------------
375
376 /// Give `real_id` a twin that shares its first eight characters.
377 ///
378 /// Eight is the floor width, so two ids only collide there in a repository far
379 /// too large to build in a test. Planting the collision is the only way to see
380 /// the widening at all, and an *archived* twin is the honest place to plant
381 /// it: archived objects are in the id set the policy is computed over and in
382 /// none of the views that list objects. A surface that abbreviates over the
383 /// rows it is showing prints eight characters here, and is wrong.
384 ///
385 /// Returns the twin's id. The real id must now be printed nine characters
386 /// wide: identical through the floor width, distinct one character later.
387 fn plant_archived_issue_twin(git: &git2::Repository, real_id: &str) -> String {
388 let mut chars: Vec<char> = real_id.chars().collect();
389 chars[8] = if chars[8] == '0' { '1' } else { '0' };
390 let twin: String = chars.into_iter().collect();
391 let target = git
392 .refname_to_id(&format!("refs/collab/issues/{}", real_id))
393 .expect("the real issue ref should exist");
394 git.reference(
395 &format!("refs/collab/archive/issues/{}", twin),
396 target,
397 false,
398 "planted twin",
399 )
400 .unwrap();
401 twin
402 }
403
404 /// The full 40-character id behind whatever prefix a command printed.
405 fn full_issue_id(git: &git2::Repository, prefix: &str) -> String {
406 git_collab::state::resolve_issue_ref(git, prefix).unwrap().1
407 }
408
409 // ---------------------------------------------------------------------------
410 // web ui
411 // ---------------------------------------------------------------------------
412
413 fn set_server_abbrev(harness: &ServerHarness, value: &str) {
414 let repo_dir = harness
415 .repos_dir()
416 .join(format!("{}.git", harness.repo_name()));
417 common::git_cmd(&repo_dir, &["config", "collab.abbrev", value]);
418 }
419
420 /// The text of the `<a>` whose href ends in `href_tail`. The list templates
421 /// link the row's id cell to the object's own page, so this is exactly the
422 /// string the page offers a reader to copy.
423 fn anchor_text(body: &str, href_tail: &str) -> String {
424 let needle = format!("{}\">", href_tail);
425 let start = body
426 .find(&needle)
427 .unwrap_or_else(|| panic!("no link ending in '{}' in:\n{}", href_tail, body))
428 + needle.len();
429 let end = body[start..]
430 .find("</a>")
431 .unwrap_or_else(|| panic!("unterminated link for '{}'", href_tail))
432 + start;
433 body[start..end].to_string()
434 }
435
436 /// The test the naive fix fails. The issue list defaults to open issues only;
437 /// the twin that forces the widening is archived and appears on none of these
438 /// pages. Its absence must not narrow the id.
439 #[test]
440 fn a_filtered_issue_page_is_abbreviated_at_the_full_set_width() {
441 let harness = ServerHarness::new("abbrev-web-filtered");
442 harness.push_head();
443
444 let short = harness.work_repo().issue_open("A visible issue");
445 let git = git2::Repository::open(harness.work_repo().dir.path()).unwrap();
446 let real_id = full_issue_id(&git, &short);
447 let twin = plant_archived_issue_twin(&git, &real_id);
448 harness.push_collab_refs();
449
450 let page = harness.get_ok(&format!("/{}/issues?filter=open", harness.repo_name()));
451 let shown = anchor_text(&page.body, &format!("/issues/{}", real_id));
452
453 assert_eq!(
454 shown.len(),
455 9,
456 "the open-issue page abbreviated over its own rows instead of every \
457 issue: showed '{}' while an archived issue shares its first eight \
458 characters",
459 shown
460 );
461 assert!(real_id.starts_with(&shown), "showed '{}'", shown);
462 assert!(
463 !twin.starts_with(&shown),
464 "'{}' names two issues, so it is not a usable reference",
465 shown
466 );
467 }
468
469 /// The same issue must render as the same string whichever filter is applied,
470 /// or the id someone copies depends on which tab they happened to be on.
471 #[test]
472 fn every_issue_filter_prints_the_same_width() {
473 let harness = ServerHarness::new("abbrev-web-agree");
474 harness.push_head();
475
476 let short = harness.work_repo().issue_open("A visible issue");
477 let git = git2::Repository::open(harness.work_repo().dir.path()).unwrap();
478 let real_id = full_issue_id(&git, &short);
479 plant_archived_issue_twin(&git, &real_id);
480 harness.push_collab_refs();
481
482 let tail = format!("/issues/{}", real_id);
483 let open = harness.get_ok(&format!("/{}/issues?filter=open", harness.repo_name()));
484 let all = harness.get_ok(&format!("/{}/issues?filter=all", harness.repo_name()));
485 let overview = harness.get_ok(&format!("/{}", harness.repo_name()));
486
487 let from_open = anchor_text(&open.body, &tail);
488 assert_eq!(from_open, anchor_text(&all.body, &tail), "open vs all");
489 assert_eq!(
490 from_open,
491 anchor_text(&overview.body, &tail),
492 "issue list vs repo overview"
493 );
494 }
495
496 #[test]
497 fn collab_abbrev_sets_the_width_of_the_web_issue_and_patch_lists() {
498 let harness = ServerHarness::new("abbrev-web-config");
499 harness.push_head();
500
501 let issue_short = harness.work_repo().issue_open("Configured issue");
502 let patch_short = harness.work_repo().patch_create("Configured patch");
503 harness.push_collab_refs();
504 set_server_abbrev(&harness, "14");
505
506 let git = git2::Repository::open(harness.work_repo().dir.path()).unwrap();
507 let issue_id = full_issue_id(&git, &issue_short);
508 let patch_id = git_collab::state::resolve_patch_ref(&git, &patch_short)
509 .unwrap()
510 .1;
511
512 let issues = harness.get_ok(&format!("/{}/issues", harness.repo_name()));
513 assert_eq!(
514 anchor_text(&issues.body, &format!("/issues/{}", issue_id)).len(),
515 14,
516 "the issue list ignored collab.abbrev"
517 );
518
519 let patches = harness.get_ok(&format!("/{}/patches", harness.repo_name()));
520 assert_eq!(
521 anchor_text(&patches.body, &format!("/patches/{}", patch_id)).len(),
522 14,
523 "the patch list ignored collab.abbrev"
524 );
525
526 let overview = harness.get_ok(&format!("/{}", harness.repo_name()));
527 assert_eq!(
528 anchor_text(&overview.body, &format!("/issues/{}", issue_id)).len(),
529 14,
530 "the repo overview ignored collab.abbrev"
531 );
532 assert_eq!(
533 anchor_text(&overview.body, &format!("/patches/{}", patch_id)).len(),
534 14,
535 "the repo overview ignored collab.abbrev"
536 );
537 }
538
539 /// `full` is git's spelling for "do not abbreviate", and the web UI has the
540 /// room to honour it.
541 #[test]
542 fn collab_abbrev_full_prints_whole_ids_in_the_web_ui() {
543 let harness = ServerHarness::new("abbrev-web-full");
544 harness.push_head();
545
546 let issue_short = harness.work_repo().issue_open("Whole id issue");
547 harness.push_collab_refs();
548 set_server_abbrev(&harness, "full");
549
550 let git = git2::Repository::open(harness.work_repo().dir.path()).unwrap();
551 let issue_id = full_issue_id(&git, &issue_short);
552
553 let issues = harness.get_ok(&format!("/{}/issues", harness.repo_name()));
554 assert_eq!(
555 anchor_text(&issues.body, &format!("/issues/{}", issue_id)),
556 issue_id,
557 "collab.abbrev=full should print the whole id"
558 );
559 }
560
561 // ---------------------------------------------------------------------------
562 // Reading is not writing
563 //
564 // Both surfaces now run an extra query — every id of a kind — on every render.
565 // It has to stay a query. A page view or a dashboard launch that appended an
566 // event or moved a ref would be writing to a shared, replicated history on
567 // behalf of someone who only looked at it.
568 // ---------------------------------------------------------------------------
569
570 /// Every collab ref and what it points at, as a sorted list.
571 fn collab_refs_snapshot(git_dir: &std::path::Path) -> Vec<String> {
572 let out = std::process::Command::new("git")
573 .args([
574 "for-each-ref",
575 "--format=%(refname) %(objectname)",
576 "refs/collab/",
577 ])
578 .current_dir(git_dir)
579 .output()
580 .expect("for-each-ref failed");
581 let mut refs: Vec<String> = String::from_utf8_lossy(&out.stdout)
582 .lines()
583 .map(|l| l.to_string())
584 .collect();
585 refs.sort();
586 refs
587 }
588
589 #[test]
590 fn rendering_web_pages_does_not_touch_the_collab_refs() {
591 let harness = ServerHarness::new("abbrev-web-readonly");
592 harness.push_head();
593 harness.work_repo().issue_open("An issue to look at");
594 harness.work_repo().patch_create("A patch to look at");
595 harness.push_collab_refs();
596
597 let served = harness
598 .repos_dir()
599 .join(format!("{}.git", harness.repo_name()));
600 let before = collab_refs_snapshot(&served);
601 assert!(!before.is_empty(), "the test needs some refs to protect");
602
603 let name = harness.repo_name();
604 for path in [
605 format!("/{}", name),
606 format!("/{}/issues", name),
607 format!("/{}/issues?filter=all", name),
608 format!("/{}/patches", name),
609 format!("/{}/patches?filter=all", name),
610 ] {
611 harness.get_ok(&path);
612 }
613
614 assert_eq!(
615 before,
616 collab_refs_snapshot(&served),
617 "rendering a page changed the collab refs"
618 );
619 }
620
621 #[test]
622 fn launching_the_dashboard_does_not_touch_the_collab_refs() {
623 let repo = TestRepo::new("Alice", "alice@example.com");
624 repo.issue_open("An issue to look at");
625 repo.patch_create("A patch to look at");
626
627 let before = collab_refs_snapshot(repo.dir.path());
628 assert!(!before.is_empty(), "the test needs some refs to protect");
629
630 // `a` cycles the status filter and `P` swaps to patches, so this walks
631 // the views that re-derive a width, not just the one on startup.
632 let output = repo.run_dashboard_smoke_sized("aPaq", 120, 24);
633 assert!(output.status.success(), "dashboard should exit cleanly");
634
635 assert_eq!(
636 before,
637 collab_refs_snapshot(repo.dir.path()),
638 "opening the dashboard changed the collab refs"
639 );
640 }
641
642 // ---------------------------------------------------------------------------
643 // patch checkout
644 //
645 // The branch `patch checkout` creates is named after the patch, so the branch
646 // name is a displayed id like any other — and the one displayed id that
647 // outlives the command by definition, since the branch stays behind.
648 // ---------------------------------------------------------------------------
649
650 /// The patch equivalent of `plant_archived_issue_twin`.
651 fn plant_archived_patch_twin(git: &git2::Repository, real_id: &str) -> String {
652 let mut chars: Vec<char> = real_id.chars().collect();
653 chars[8] = if chars[8] == '0' { '1' } else { '0' };
654 let twin: String = chars.into_iter().collect();
655 let target = git
656 .refname_to_id(&format!("refs/collab/patches/{}/events", real_id))
657 .expect("the real patch ref should exist");
658 git.reference(
659 &format!("refs/collab/archive/patches/{}/events", twin),
660 target,
661 false,
662 "planted twin",
663 )
664 .unwrap();
665 twin
666 }
667
668 /// Two patches sharing eight characters must not want the same branch name.
669 ///
670 /// The suffix loop in `checkout` would paper over it — `collab/<id>-1` — but
671 /// that name reads as a second checkout of one patch rather than a checkout of
672 /// a different one, and the reviewer has no way to tell which patch they are
673 /// looking at from the branch they are on.
674 #[test]
675 fn patch_checkout_names_the_branch_at_the_full_set_width() {
676 let repo = TestRepo::new("Alice", "alice@example.com");
677 let short = repo.patch_create("A patch to check out");
678 let git = git2::Repository::open(repo.dir.path()).unwrap();
679 let real_id = git_collab::state::resolve_patch_ref(&git, &short)
680 .unwrap()
681 .1;
682 let twin = plant_archived_patch_twin(&git, &real_id);
683
684 let out = repo.run_ok(&["patch", "checkout", &real_id]);
685 let branch = repo.git(&["rev-parse", "--abbrev-ref", "HEAD"]);
686 let branch = branch.trim();
687
688 assert_eq!(
689 branch,
690 format!("collab/{}", &real_id[..9]),
691 "the branch name was cut to a prefix that also names patch {}",
692 &twin[..9]
693 );
694 assert!(
695 out.contains(&real_id[..9]),
696 "the message should name the patch at the same width: {}",
697 out
698 );
699 }
700
701 #[test]
702 fn collab_abbrev_sets_the_width_of_the_patch_checkout_branch() {
703 let repo = TestRepo::new("Alice", "alice@example.com");
704 let short = repo.patch_create("A configured patch");
705 let git = git2::Repository::open(repo.dir.path()).unwrap();
706 let real_id = git_collab::state::resolve_patch_ref(&git, &short)
707 .unwrap()
708 .1;
709 set_abbrev(&repo, "14");
710
711 repo.run_ok(&["patch", "checkout", &real_id]);
712 let branch = repo.git(&["rev-parse", "--abbrev-ref", "HEAD"]);
713
714 assert_eq!(branch.trim(), format!("collab/{}", &real_id[..14]));
715 }
716
717 // ---------------------------------------------------------------------------
718 // dashboard
719 //
720 // The TUI is driven through a pty, so what these assert on is the characters
721 // that actually reached the screen.
722 // ---------------------------------------------------------------------------
723
724 /// The printable text of a terminal session, with the escape sequences that
725 /// carried the colours removed.
726 fn strip_ansi(raw: &str) -> String {
727 let mut out = String::new();
728 let mut chars = raw.chars().peekable();
729 while let Some(c) = chars.next() {
730 if c != '\u{1b}' {
731 out.push(c);
732 continue;
733 }
734 // CSI sequences run until a byte in @-~; anything else is a two-byte
735 // escape. Neither ever contains printable payload the dashboard means
736 // for a reader, so both are dropped whole.
737 if chars.peek() == Some(&'[') {
738 chars.next();
739 for c2 in chars.by_ref() {
740 if ('@'..='~').contains(&c2) {
741 break;
742 }
743 }
744 } else {
745 chars.next();
746 }
747 }
748 out
749 }
750
751 fn dashboard_screen(repo: &TestRepo, cols: u16) -> String {
752 let output = repo.run_dashboard_smoke_sized("q", cols, 24);
753 assert!(
754 output.status.success(),
755 "dashboard did not exit cleanly: {}",
756 String::from_utf8_lossy(&output.stderr)
757 );
758 strip_ansi(&String::from_utf8_lossy(&output.stdout))
759 }
760
761 /// Open one issue and give it an archived twin sharing its first eight
762 /// characters. Returns the issue's full id.
763 fn issue_with_archived_twin(repo: &TestRepo, title: &str) -> (String, String) {
764 let short = repo.issue_open(title);
765 let git = git2::Repository::open(repo.dir.path()).unwrap();
766 let real_id = full_issue_id(&git, &short);
767 let twin = plant_archived_issue_twin(&git, &real_id);
768 (real_id, twin)
769 }
770
771 /// The dashboard's counterpart to the web test above. Its issue list defaults
772 /// to open issues and never shows archived ones at all, yet the archived twin
773 /// still has to widen what it prints.
774 #[test]
775 fn the_dashboard_issue_list_is_abbreviated_at_the_full_set_width() {
776 let repo = TestRepo::new("Alice", "alice@example.com");
777 let (real_id, twin) = issue_with_archived_twin(&repo, "Dashboard issue");
778
779 let screen = dashboard_screen(&repo, 120);
780
781 assert!(
782 screen.contains(&format!("{} ", &real_id[..9])),
783 "expected the issue id widened to nine characters ('{}') on screen:\n{}",
784 &real_id[..9],
785 screen
786 );
787 assert!(
788 !screen.contains(&format!("{} ", &real_id[..8])),
789 "the dashboard printed '{}', which also names the archived issue {}",
790 &real_id[..8],
791 &twin[..9]
792 );
793 }
794
795 #[test]
796 fn collab_abbrev_sets_the_width_of_the_dashboard_lists() {
797 let repo = TestRepo::new("Alice", "alice@example.com");
798 let short = repo.issue_open("Configured issue");
799 let git = git2::Repository::open(repo.dir.path()).unwrap();
800 let issue_id = full_issue_id(&git, &short);
801 set_abbrev(&repo, "14");
802
803 let screen = dashboard_screen(&repo, 120);
804 assert!(
805 screen.contains(&format!("{} ", &issue_id[..14])),
806 "the dashboard ignored collab.abbrev=14:\n{}",
807 screen
808 );
809 }
810
811 #[test]
812 fn collab_abbrev_full_prints_whole_ids_in_the_dashboard() {
813 let repo = TestRepo::new("Alice", "alice@example.com");
814 let short = repo.issue_open("Whole id issue");
815 let git = git2::Repository::open(repo.dir.path()).unwrap();
816 let issue_id = full_issue_id(&git, &short);
817 set_abbrev(&repo, "full");
818
819 // Wide enough that a 40-character id is not merely clipped by the pane.
820 let screen = dashboard_screen(&repo, 200);
821 assert!(
822 screen.contains(&issue_id),
823 "collab.abbrev=full should print the whole id '{}':\n{}",
824 issue_id,
825 screen
826 );
827 }
tests/common/mod.rs
Old New
@@ -572,12 +572,27 @@ impl TestRepo {
572 572
573 /// Run `git-collab dashboard` in a pseudo-terminal, feed input, and return raw output. 573 /// Run `git-collab dashboard` in a pseudo-terminal, feed input, and return raw output.
574 pub fn run_dashboard_smoke(&self, input: &str) -> Output { 574 pub fn run_dashboard_smoke(&self, input: &str) -> Output {
575 self.run_dashboard_smoke_sized(input, 80, 24)
576 }
577
578 /// As `run_dashboard_smoke`, but with an explicit terminal size.
579 ///
580 /// The dashboard lays its panes out in columns and clips whatever does not
581 /// fit, so a test asserting on a rendered id has to give it room for the
582 /// widest id it could legitimately print. `stty` runs inside the pty
583 /// `script` allocates, before the dashboard reads its size.
584 pub fn run_dashboard_smoke_sized(&self, input: &str, cols: u16, rows: u16) -> Output {
575 let mut command = Command::new("script"); 585 let mut command = Command::new("script");
576 self.apply_env(&mut command); 586 self.apply_env(&mut command);
577 let mut child = command 587 let mut child = command
578 .args([ 588 .args([
579 "-qec", 589 "-qec",
580 &format!("{} dashboard", env!("CARGO_BIN_EXE_git-collab")), 590 &format!(
591 "stty cols {} rows {}; {} dashboard",
592 cols,
593 rows,
594 env!("CARGO_BIN_EXE_git-collab")
595 ),
581 "/dev/null", 596 "/dev/null",
582 ]) 597 ])
583 .current_dir(self.dir.path()) 598 .current_dir(self.dir.path())