Re: [pve-devel] [PATCH v3 storage 1/2] fix #3633: pvesm: use print_api_result

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 25/10/2021 16:17, Dominik Csapak wrote:
> both patches LGTM, commit message could have used some more info
> (like which fields were added, etc.) but no big problem

yeah, mentioning that instead of three or so selected columns all are now
dumped and some reasoning for doing that (after all #3633 did not asked for that)
would be good ;)

Would be nice to reduce that a bit for the default output, ideally the content
list would made dependent of the content type passed, as `used`, `vmid`, `parent`,
`encrypted` and `verification` do not make much sense for `--content vztmpl`.

The headers are now all the internal property, `used_fraction` does not read
to nicely for a CLI tool human readable output IMO (would be naturally Ok for
the machine readable JSON one).

There's also ctime in content list that isn't printed more human friendly, which
would be the point of #3633,

And we really need to reverse the default for (no)border, as the content list
for vztmpl on the same storage here produced 72 lines previously (easily fitting
on one console page here) and now spews out 145 lines (which needs scrolling),
without adding really any value and making copying not exactly easier, but at
least that part isn't the job of this patch series.

> Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
> Tested-by: Dominik Csapak<d.csapak@proxmox.com>

thanks, it works as in breaks not something, but cannot say as advertised as it
does quite a bit more than the report requested, which can be fine but would need
some commit message to state at least some reasoning or thoughts on it.

I'm open to take it in now with at least the columns being the same as previously,
fancy auto-column selection and the border stuff can be done later/independent of
this.