doctl icon indicating copy to clipboard operation
doctl copied to clipboard

Droplet: Add SizeSlug as format flag

Open danaelhe opened this issue 1 year ago • 3 comments

Includes SizeSlug as a format flag in help docs:

go run cmd/doctl/main.go compute droplet list -h

....

Flags:
      --format ID         Columns for output in a comma-separated list. Possible values: ID, `Name`, `SizeSlug`, `PublicIPv4`, `PrivateIPv4`, `PublicIPv6`, `Memory`, `VCPUs`, `Disk`, `Region`, `Image`, `VPCUUID`, `Status`, `Tags`, `Features`, `Volumes`.
  -h, --help              help for list
      --no-header         Return raw data with no headers
      --region string     Retrieves a list of Droplets in a specified region
      --tag-name string   Retrieves a list of Droplets with the specified tag name

danaelhe avatar Aug 13 '24 17:08 danaelhe

Ooop need to update some tests too, one sec

danaelhe avatar Aug 13 '24 17:08 danaelhe

Tests are failing because it now includes SizeSlug in output by default:

               	Error:      	Not equal: 
                	            	expected: "ID      Name                 Public IPv4    Private IPv4    Public IPv6    Memory    VCPUs    Disk    Region              Image                          VPC UUID    Status    Tags    Features    Volumes\n5555    some-droplet-name                                                  0         0        0       some-region-slug    some-distro some-image-name                active    yes     remotes     some-volume-id"
                	            	actual  : "ID      Name                 Size Slug    Public IPv4    Private IPv4    Public IPv6    Memory    VCPUs    Disk    Region              Image                          VPC UUID    Status    Tags    Features    Volumes\n5555    some-droplet-name                                                               0         0        0       some-region-slug    some-distro some-image-name                active    yes     remotes     some-volume-id"
                	            	
                	            	Diff:
                	            	--- Expected
                	            	+++ Actual
                	            	@@ -1,2 +1,2 @@
                	            	-ID      Name                 Public IPv4    Private IPv4    Public IPv6    Memory    VCPUs    Disk    Region              Image                          VPC UUID    Status    Tags    Features    Volumes
                	            	-5555    some-droplet-name                                                  0         0        0       some-region-slug    some-distro some-image-name                active    yes     remotes     some-volume-id
                	            	+ID      Name                 Size Slug    Public IPv4    Private IPv4    Public IPv6    Memory    VCPUs    Disk    Region              Image                          VPC UUID    Status    Tags    Features    Volumes
                	            	+5555    some-droplet-name                                                               0         0        0       some-region-slug    some-distro some-image-name                active    yes     remotes     some-volume-id

I can update the tests if this is not considered a breaking change? @andrewsomething

danaelhe avatar Aug 14 '24 16:08 danaelhe

Adding new columns isn't necessarily a breaking change. For scripting, we should be encouraging usage of the --format flag to pick the specific values the user cares about and provide a stable order. Though we also need to think about readability of the default output. Unfortunately, the output here is already too wide to fit on a single line in most cases. While the slug is useful, it does duplicate some of the info already shown.

I wonder if there is some more general solution for non-default columns? There are a lot of commands that could benefit.

The values shown in the help output come from here:

https://github.com/digitalocean/doctl/blob/9282784517cb25d3cb6b06a4895626b173177603/commands/command_option.go#L35

It might be as simple as:

-               c.fmtCols = d.Cols()
+               c.fmtCols = maps.Keys(d.ColMap())

Though we need to check that the change doesn't have any undesired side effects.

andrewsomething avatar Aug 14 '24 20:08 andrewsomething