From c6ac54a76db039bbede73d604be6584157dc7db6 Mon Sep 17 00:00:00 2001 From: Eliza Weisman Date: Wed, 6 Nov 2024 13:15:21 -0800 Subject: [PATCH] [omdb] Display (some) name columns last in tables (#6983) Presently, OMDB lists the names of instances and disks towards the left-hand of tabular output. This is probably because instance/disk names are relatively _important_ to the user, so it makes sense to put them first. However, user-defined string names also wildly vary in length, in contrast to many other columns in OMDB output, such as UUIDs, timestamps, etc, which are broadly fixed width. Putting names "early" (from a left-to-right perspective) means that when there are a couple names in the list that are much longer than the majority of names, most lines have a lot of whitespace to pad the short names to the same length as the long ones. This wastes horizontal characters and makes the output more likely to wrap on reasonably sized terminals. Putting the name column last makes it likelier that only those names that are very long will line-wrap, and most lines will not wrap, which makes tabular output much more intelligible. For example, consider the `omdb db instance ls` output. Note that I'm using screenshots here to demonstrate the difference in line-wrapping behavior. On `main`, with the name in the second column, all the lines have been wrapped: ![Screenshot from 2024-11-01 13-34-46](https://github.com/user-attachments/assets/478c4f4d-c76e-48cc-8dbc-2d1d6f847b22) On this branch, with the name in the last column, none of the lines in the first screenful of output have wrapped: ![image](https://github.com/user-attachments/assets/592a7b41-4480-4ad0-9c88-ab4b04125800) Scrolling down a bit reveals that a couple of instances do have names that are long enough to wrap the line, but the line wrapping only effects the lines for *those* instances. Since the name column for other instances doesn't need to be padded to that length, most lines in the table are still intelligible: ![image](https://github.com/user-attachments/assets/c58f6a12-e1b1-4b7e-bb9e-22f1969f270d) --- dev-tools/omdb/src/bin/omdb/db.rs | 15 ++++++--------- dev-tools/omdb/tests/successes.out | 4 ++-- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/db.rs b/dev-tools/omdb/src/bin/omdb/db.rs index 9c43d0d670..d971545087 100644 --- a/dev-tools/omdb/src/bin/omdb/db.rs +++ b/dev-tools/omdb/src/bin/omdb/db.rs @@ -1299,10 +1299,10 @@ async fn lookup_project( #[derive(Tabled)] #[tabled(rename_all = "SCREAMING_SNAKE_CASE")] struct DiskIdentity { - name: String, id: Uuid, size: String, state: String, + name: String, } impl From<&'_ db::model::Disk> for DiskIdentity { @@ -3325,17 +3325,14 @@ async fn cmd_db_instance_info( println!("\n{:=<80}", "== ATTACHED DISKS "); check_limit(&disks, fetch_opts.fetch_limit, ctx); - let table = if fetch_opts.include_deleted { + let mut table = if fetch_opts.include_deleted { tabled::Table::new(disks.iter().map(MaybeDeletedDiskRow::from)) - .with(tabled::settings::Style::empty()) - .with(tabled::settings::Padding::new(0, 1, 0, 0)) - .to_string() } else { tabled::Table::new(disks.iter().map(DiskRow::from)) - .with(tabled::settings::Style::empty()) - .with(tabled::settings::Padding::new(0, 1, 0, 0)) - .to_string() }; + table + .with(tabled::settings::Style::empty()) + .with(tabled::settings::Padding::new(0, 1, 0, 0)); println!("{table}"); } @@ -3502,11 +3499,11 @@ struct VmmStateRow { #[tabled(rename_all = "SCREAMING_SNAKE_CASE")] struct CustomerInstanceRow { id: String, - name: String, state: String, propolis_id: MaybePropolisId, sled_id: MaybeSledId, host_serial: String, + name: String, } /// Run `omdb db instances`: list data about customer VMs. diff --git a/dev-tools/omdb/tests/successes.out b/dev-tools/omdb/tests/successes.out index 1e99dbd3a8..6974c0b36b 100644 --- a/dev-tools/omdb/tests/successes.out +++ b/dev-tools/omdb/tests/successes.out @@ -2,7 +2,7 @@ EXECUTING COMMAND: omdb ["db", "disks", "list"] termination: Exited(0) --------------------------------------------- stdout: -NAME ID SIZE STATE ATTACHED_TO +ID SIZE STATE NAME ATTACHED_TO --------------------------------------------- stderr: note: using database URL postgresql://root@[::1]:REDACTED_PORT/omicron?sslmode=disable @@ -52,7 +52,7 @@ EXECUTING COMMAND: omdb ["db", "instances"] termination: Exited(0) --------------------------------------------- stdout: -ID NAME STATE PROPOLIS_ID SLED_ID HOST_SERIAL +ID STATE PROPOLIS_ID SLED_ID HOST_SERIAL NAME --------------------------------------------- stderr: note: using database URL postgresql://root@[::1]:REDACTED_PORT/omicron?sslmode=disable