Skip to content

Conversation

@nordlundj
Copy link
Contributor

This pull request refactors the way tabular data is printed in several CLI commands by introducing a new reusable table printer utility. Previously, each command manually calculated column widths and formatted output using fmt.Printf. Now, all commands use the new table package for dynamic and consistent table formatting, improving maintainability and output readability.

Table printing improvements:

  • Added the new internal/table/table.go utility, which provides a Printer type for dynamic column width calculation and formatted output, along with a corresponding test suite in internal/table/table_test.go. [1] [2]

Refactoring CLI commands to use the new table printer:

  • Updated cmd/instance_list.go, cmd/instance_nodes.go, cmd/instance_plugins.go, cmd/plans.go, cmd/team_list.go, and cmd/vpc_list.go to replace manual table formatting with the new table printer, resulting in cleaner and more consistent code. [1] [2] [3] [4] [5] [6] [7]

Codebase modernization:

  • Added necessary imports for os and the new table package to affected files, removing unused imports such as encoding/json where appropriate. [1] [2] [3] [4] [5] [6]

Output consistency and feature enhancements:

  • Enhanced output by automatically adjusting column widths and adding support for new columns (e.g., roles and 2FA status in team listing, plan price formatting), ensuring all tables have headers and separators. [1] [2]

Testing:

  • Added unit tests for the table printer to verify correct alignment and error handling for column mismatches.

@nordlundj nordlundj requested a review from Copilot November 25, 2025 14:03
Copilot finished reviewing on behalf of nordlundj November 25, 2025 14:05
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request introduces a reusable table printer utility to standardize and simplify tabular output across CLI commands. Previously, each command manually calculated column widths and formatted output, leading to code duplication and inconsistent formatting.

  • Introduces a new internal/table package with dynamic column width calculation
  • Refactors six CLI commands (instance_list, instance_nodes, instance_plugins, plans, team_list, vpc_list) to use the new table printer
  • Enhances output with new columns (roles and 2FA status in team listing, VPC ID, improved plan price formatting)

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
internal/table/table.go New table printer utility with automatic width calculation and formatted output
internal/table/table_test.go Unit tests for table printer functionality including alignment and error handling
cmd/instance_list.go Replaces manual table formatting with table printer, reducing code from ~50 to ~15 lines
cmd/instance_nodes.go Adopts table printer for node listing with improved disk size formatting
cmd/instance_plugins.go Uses table printer for plugin listing output
cmd/plans.go Switches from JSON output to formatted table with price and sharing information
cmd/team_list.go Adds new ROLES and 2FA columns using table printer
cmd/vpc_list.go Adds VPC ID column and uses table printer for consistent formatting

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nordlundj
Copy link
Contributor Author

In response to the Copilot comments on checking the Error returned by AddRow:

We control both the table creation AND the row additions - they're in the same function, just a few lines apart
The column count is statically known at compile time (we pass string literals)
If we mess up the column count, we'll catch it immediately in testing/development - the table will look obviously broken
Adding error handling for every AddRow call adds 3-4 lines of boilerplate per row, making the code much more verbose
The error can only happen due to programmer error (wrong number of args), not runtime conditions

@nordlundj nordlundj marked this pull request as ready for review November 25, 2025 14:28
@dentarg dentarg merged commit 285bd2b into main Nov 25, 2025
3 checks passed
@dentarg dentarg deleted the generic-table-output branch November 25, 2025 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants