Skip to main content

max / goingson

Name the columns in the import preview and the task list csv_rows already returned its columns and its rows together, which is the shape naming wants: one function decides both. The task list's row builder sat in a different function from its column list, which is the other case where position drifts unnoticed.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session
https://claude.ai/code/session_01MptwXZ8k65v19rFmdGAyki
Author: Max Johnson <me@maxj.phd> · 2026-09-03 03:26 UTC
Signed with PGP, not checked
Commit: 0588ebda554e4480db97472bf47bb1dfb7bacbb9
Parent: e7fd578
2 files changed, +103 insertions, -71 deletions
@@ -82,7 +82,7 @@
82 82
83 83 use goingson_core::ImportOptions;
84 84 use makeover_layout::{FieldKind, Tone};
85 - use quasi_router::screen::{Accepted, Act, Cells, Choice, Column, Field, Row};
85 + use quasi_router::screen::{Accepted, Act, Cells, Choice, Column, Field, Row, Table};
86 86 use quasi_router::{Action, Node, RegionKind, Response, RouteError, Router, Screen, Slot};
87 87
88 88 use crate::backup_scheduler::backup_dir;
@@ -281,16 +281,18 @@
281 281 }
282 282
283 283 /// The preview table, and the line saying what it left out.
284 + ///
285 + /// Two of the three callers hand rows that name their columns and the third
286 + /// hands positional ones; the table takes either, since a row that named
287 + /// nothing is passed through as it was written.
284 288 fn preview_table(columns: &[&str], rows: Vec<Cells>, total: usize) -> Vec<Node> {
285 - let mut out = vec![Node::Table {
286 - columns: columns.iter().map(|name| Column::new(*name)).collect(),
287 - rows,
288 - // A parsed file is not a page of a query: every row is already in hand,
289 - // and the 25 shown are a reading convenience rather than a window that
290 - // could be widened. `Rest` would describe an address that fetches more,
291 - // and there is none.
292 - more: None,
293 - }];
289 + // No `more`, and that is a statement rather than an omission: a parsed file
290 + // is not a page of a query. Every row is already in hand, and the 25 shown
291 + // are a reading convenience rather than a window that could be widened. A
292 + // `Rest` would describe an address that fetches more, and there is none.
293 + let mut out = vec![Node::from(
294 + Table::new(columns.iter().map(|name| Column::new(*name))).rows(rows),
295 + )];
294 296 if total > PREVIEW_ROWS {
295 297 out.push(Node::text(format!(
296 298 "Showing the first {PREVIEW_ROWS} of {total}.",
@@ -372,11 +374,16 @@
372 374 Ok(preview_region(nodes))
373 375 }
374 376
375 - /// The columns a parsed CSV shows, and its rows in that order.
377 + /// The columns a parsed CSV shows, and its rows against them.
376 378 ///
377 379 /// The shipped `getColumnsForEntityType` keys into the item's camelCase `data`
378 380 /// object; here the parse is already typed, so a column is a match arm rather
379 381 /// than a string key that can miss.
382 + ///
383 + /// Both halves are conditional on the same entity type, and until the cells
384 + /// named their columns the two match arms had to agree on order with nothing
385 + /// checking that they did. Now the column list is the only thing that decides
386 + /// where a value lands, and the row arm only has to spell the heading.
380 387 fn csv_rows(parsed: &goingson_core::ImportParseResult) -> (Vec<&'static str>, Vec<Cells>) {
381 388 use goingson_core::ImportItemData;
382 389
@@ -392,24 +399,30 @@
392 399 .iter()
393 400 .take(PREVIEW_ROWS)
394 401 .map(|item| match &item.data {
395 - ImportItemData::Task(task) => Cells::new([
396 - short(&task.description),
397 - short(task.project_name.as_ref().unwrap_or(&blank)),
398 - short(task.priority.as_ref().unwrap_or(&blank)),
399 - short(task.due.as_ref().unwrap_or(&blank)),
400 - ]),
401 - ImportItemData::Project(project) => Cells::new([
402 - short(&project.name),
403 - short(project.description.as_ref().unwrap_or(&blank)),
404 - short(project.project_type.as_ref().unwrap_or(&blank)),
405 - short(project.status.as_ref().unwrap_or(&blank)),
406 - ]),
407 - ImportItemData::Event(event) => Cells::new([
408 - short(&event.title),
409 - short(&event.start),
410 - short(event.end.as_ref().unwrap_or(&blank)),
411 - short(event.location.as_ref().unwrap_or(&blank)),
412 - ]),
402 + ImportItemData::Task(task) => Cells::default()
403 + .at("Description", short(&task.description))
404 + .at(
405 + "Project",
406 + short(task.project_name.as_ref().unwrap_or(&blank)),
407 + )
408 + .at("Priority", short(task.priority.as_ref().unwrap_or(&blank)))
409 + .at("Due", short(task.due.as_ref().unwrap_or(&blank))),
410 + ImportItemData::Project(project) => Cells::default()
411 + .at("Name", short(&project.name))
412 + .at(
413 + "Description",
414 + short(project.description.as_ref().unwrap_or(&blank)),
415 + )
416 + .at(
417 + "Type",
418 + short(project.project_type.as_ref().unwrap_or(&blank)),
419 + )
420 + .at("Status", short(project.status.as_ref().unwrap_or(&blank))),
421 + ImportItemData::Event(event) => Cells::default()
422 + .at("Title", short(&event.title))
423 + .at("Start", short(&event.start))
424 + .at("End", short(event.end.as_ref().unwrap_or(&blank)))
425 + .at("Location", short(event.location.as_ref().unwrap_or(&blank))),
413 426 })
414 427 .collect();
415 428
@@ -437,23 +450,28 @@
437 450 .iter()
438 451 .take(PREVIEW_ROWS)
439 452 .map(|card| {
440 - let mut status = Cells::new([
441 - short(&card.display_name),
442 - short(card.company.as_deref().unwrap_or_default()),
443 - card.email_count.to_string(),
444 - card.phone_count.to_string(),
445 - ]);
446 - // The shipped cell says "Already exists" and hides which contact it
447 - // matched in a `title=`. The name is the useful half and it is a
448 - // fact, so it is said.
449 - status.values.push(
450 - card.duplicate_of
451 - .as_ref()
452 - .map_or_else(quasi_router::screen::Cell::default, |existing| {
453 - quasi_router::screen::Cell::new(format!("Matches {existing}"))
454 - }),
455 - );
456 - status
453 + Cells::default()
454 + .at("Name", short(&card.display_name))
455 + .at(
456 + "Company",
457 + short(card.company.as_deref().unwrap_or_default()),
458 + )
459 + .at("Emails", card.email_count.to_string())
460 + .at("Phones", card.phone_count.to_string())
461 + // The shipped cell says "Already exists" and hides which
462 + // contact it matched in a `title=`. The name is the useful half
463 + // and it is a fact, so it is said. Named rather than pushed
464 + // onto the end of a positional row: a push says "after the
465 + // others", which is only the Status column while the four
466 + // before it are written in exactly this order.
467 + .at(
468 + "Status",
469 + card.duplicate_of
470 + .as_ref()
471 + .map_or_else(quasi_router::screen::Cell::default, |existing| {
472 + quasi_router::screen::Cell::new(format!("Matches {existing}"))
473 + }),
474 + )
457 475 })
458 476 .collect();
459 477
@@ -493,6 +511,9 @@
493 511 let rows = events
494 512 .iter()
495 513 .take(PREVIEW_ROWS)
514 + // Positional, and left that way: the four headings are handed to
515 + // `preview_table` a dozen lines below, so the columns and the cells are
516 + // read together and every row has all four.
496 517 .map(|event| {
497 518 Cells::new([
498 519 short(&event.title),
@@ -30,7 +30,7 @@
30 30 //! build-time one cannot. Nothing checks that the two agree, so add a column in
31 31 //! both by hand.
32 32 //!
33 - //! `Node::Table` carries a [`Rest`](quasi_router::screen::Rest), which is a
33 + //! A [`Table`] carries a [`Rest`](quasi_router::screen::Rest), which is a
34 34 //! `layout::Paging`: the description carries where the reader is rather than
35 35 //! merely that there is more. At `PAGE * PAGES` there is no address that would
36 36 //! show anything new, so the ceiling is a sentence rather than a control, and
@@ -92,7 +92,7 @@
92 92 };
93 93 use makeover_layout::{Sort, Tone, Width};
94 94 use quasi_router::screen::{
95 - Act, Cell, Cells, Choice, Column, Consult, Field, Figure, Meter, Rest, Tag,
95 + Act, Cell, Cells, Choice, Column, Consult, Field, Figure, Meter, Rest, Table, Tag,
96 96 };
97 97 use quasi_router::{Action, Node, RegionKind, Response, RouteError, Router, Screen, Slot};
98 98
@@ -606,7 +606,13 @@
606 606 acts
607 607 }
608 608
609 - /// One task as a row of cells, positional against [`COLUMNS`].
609 + /// One task as a row of cells, each naming the column it belongs to.
610 + ///
611 + /// The names are [`COLUMNS`]'s own words, resolved by [`Table::row`] against
612 + /// the columns [`columns`] built from that same constant. Every row does carry
613 + /// all seven cells, so position would land them correctly today; what it would
614 + /// not survive is the two lists living in different functions, where the only
615 + /// thing holding them in the same order is somebody reading both.
610 616 fn row_for(task: &Task, view: &View) -> Cells {
611 617 let progress = if task.subtask_count() > 0 {
612 618 Cell::default().part(Node::Meter(Meter {
@@ -627,25 +633,30 @@
627 633 Tone::Neutral
628 634 }));
629 635
630 - Cells::new([
631 - description_cell(task),
632 - Cell::new(task.project_name_or_dash()),
636 + Cells::default()
637 + .at("description", description_cell(task))
638 + .at("project", Cell::new(task.project_name_or_dash()))
633 639 // The single letter the shipped column shows, which is the whole cell.
634 640 // `as_str` is the right method here and only here: this is the
635 641 // one-character column it was written for.
636 - Cell::tag(Tag::badge(task.priority.as_str()).tone(priority_tone(&task.priority))),
637 - due,
638 - Cell::new(if task.has_recurrence() {
639 - task.recurrence.as_str()
640 - } else {
641 - "-"
642 - }),
643 - progress,
644 - Cell::acts(acts_for(task, view)),
645 - ])
646 - // The row joins the screen's selection under its own id, which is what the
647 - // bulk bar acts on.
648 - .ticking(task.id.to_string(), view.ticked)
642 + .at(
643 + "priority",
644 + Cell::tag(Tag::badge(task.priority.as_str()).tone(priority_tone(&task.priority))),
645 + )
646 + .at("due", due)
647 + .at(
648 + "recurrence",
649 + Cell::new(if task.has_recurrence() {
650 + task.recurrence.as_str()
651 + } else {
652 + "-"
653 + }),
654 + )
655 + .at("progress", progress)
656 + .at("actions", Cell::acts(acts_for(task, view)))
657 + // The row joins the screen's selection under its own id, which is what
658 + // the bulk bar acts on.
659 + .ticking(task.id.to_string(), view.ticked)
649 660 }
650 661
651 662 /// The controls over the selection.
@@ -816,7 +827,7 @@
816 827
817 828 /// The table, and the way to more of it.
818 829 ///
819 - /// The table says there is more itself, through `Node::Table`'s `Rest`. The
830 + /// The table says there is more itself, through [`Table::more`]. The
820 831 /// second node this can return is the ceiling sentence: at `PAGE * PAGES` there is
821 832 /// no address left to offer and the honest answer is a sentence.
822 833 fn table(state: &AppState, view: &View) -> Result<Vec<Node>, RouteError> {
@@ -845,11 +856,11 @@
845 856 .of(usize::try_from(total).unwrap_or(usize::MAX))
846 857 });
847 858
848 - let mut out = vec![Node::Table {
849 - columns: columns(view),
850 - rows: tasks.iter().map(|task| row_for(task, view)).collect(),
851 - more,
852 - }];
859 + let mut table = Table::new(columns(view)).rows(tasks.iter().map(|task| row_for(task, view)));
860 + if let Some(rest) = more {
861 + table = table.more(rest);
862 + }
863 + let mut out = vec![Node::from(table)];
853 864
854 865 // At the ceiling there is no address that would show anything new, so the
855 866 // honest sentence is offered instead of a control that asks for the rows