CLI configuration option for default table layout

[harridu]

IMHO the ASCII graphics added to the tables provided by various incus commands is pretty much distracting. Some say, its a waste of precious screen space. I would highly appreciate an environment variable or a central config option to make "-f compact" the default, even for the tables hidden deep inside the output of various incus subcommands, eg the table of snapshots for "incus info containername".

Hopefully I am not too blind to see. I am new to Incus, and I haven't found the man page for .config/incus/config.yml yet.

Required information

• Distribution: Debian
• Distribution version: 12.6
• The output of "incus info" or if that fails:
  • Kernel version: Linux srvl051.example.com 6.7.12+bpo-amd64 SMP PREEMPT_DYNAMIC Debian 6.7.12-1~bpo12+1 (2024-05-06) x86_64 GNU/Linux
  • LXC version: 1:5.0.3-2~xgo120+1
  • Incus version: 6.0.0-1~bpo12+1
  • Storage backend in use: lvm

[stgraber]

There is a way to do it through aliases.

incus alias add list "list -fcompact"

[harridu]

root@srvl051:~# incus alias add info "info -fcompact"
root@srvl051:~# incus info first
Error: unknown shorthand flag: 'f' in -fcompact

As written above, a central config option affecting all tables would be nice.

[smemsh]

for that matter, if the boxes used utf-8 linedraw chars, they would be a lot easier on the eyes... maybe also helpful if the outside borders were optional.

[stgraber]

We're basically just consuming https://github.com/olekukonko/tablewriter

[mjf]

@stgraber I would also love to see output example 10 as default for tables or a global configuration option (the easiest to implement, I guess). To setup all the aliases (which I did) is truly suboptimal.

[arojas2003]

Hi there! My partner @tonyn10 and I in UT Austin's Virtualization class would like to tackle this issue.

[tonyn10]

Hi, I am @arojas2003's partner.

[stgraber]

For this one, I expect we'll want a change to shared/cliconfig so we have somewhere to store those kind of settings. Probably a Defaults section within the config struct or something along those lines.

We can then have a field within such a Defaults struct for ListFormat which one would then be able to set to compact for example, having it then get picked up by all list commands.

[tonyn10]

Hi @stgraber ,

It looks like @rxtom has an open pull request in a forked repo https://github.com/rxtom/incus/pull/2 that perhaps addresses this issue.

Is there any further action on our part that needs to be done?

[stgraber]

That commit isn't signed off and wasn't sent to this repo so we can't really consider it.

[rxtom]

Hi @stgraber ,

It looks like @rxtom has an open pull request in a forked repo rxtom#2 that perhaps addresses this issue.

Is there any further action on our part that needs to be done?

@tonyn10 That commit is incomplete. Feel free to include / discard any parts of it.

[stgraber]

For now I'd keep it simple and stick to what I described in https://github.com/lxc/incus/issues/969#issuecomment-2776820149

Once we have the extra config it will be easy to add in more options in the future.

[tonyn10]

@stgraber Just to confirm, we just want modify all the list commands that have been using "table" and "compact" as the default flag format, to pull from the config default struct we construct (which will have ListFormat = "compact")?

We want to leave the other list commands who are using "yaml" and some other list format unchanged?

In other words, what types of list commands should we be interested in using the defaults struct?

[stgraber]

stgraber@castiana:~/Code/lxc/incus (git:stgraber/main)$ git grep flagFormat cmd/incus | grep table
cmd/incus/admin_sql.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/alias.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/cluster.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/cluster.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable if demanded, e.g. csv,header`)+"``")
cmd/incus/cluster_group.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/config_template.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/config_trust.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/config_trust.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/image.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/image_alias.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/list.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/network.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/network.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/network_acl.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/network_address_set.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G("Format (csv|json|table|yaml|compact)")+"``")
cmd/incus/network_allocations.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/network_forward.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/network_integration.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/network_load_balancer.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/network_peer.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/network_zone.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/network_zone.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/operation.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/profile.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/project.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/project.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/remote.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/snapshot.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/storage.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/storage_bucket.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/storage_bucket.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/storage_volume.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/storage_volume.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")
cmd/incus/top.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G("Format (table|compact)")+"``")
cmd/incus/warning.go:	cmd.Flags().StringVarP(&c.flagFormat, "format", "f", "table", i18n.G(`Format (csv|json|table|yaml|compact), use suffix ",noheader" to disable headers and ",header" to enable it if missing, e.g. csv,header`)+"``")

All of those should have the default value of "table" which is currently hardcoded be instead pulled from the new configuration key with it default to "table" if it's empty/not-set in config.

Easiest is probably to add a function to cmdGlobal, something like:

func (c *cmdGlobal) defaultListFormat() string {
    if c.conf != nil || c.conf.Defaults.ListFormat == "" {
        return "table"
    }

    return c.conf.Defaults.ListFormat
}

Then every one of those cmd.Flags().StringVarP listed above can have its fixed "table" be replaced by c.global.defaultListFormat() instead.

[tonyn10]

Hi @stgraber ,

We already implemented the items you mentioned above this thread:

• creating the Defaults struct with a ListFormat field defaulted to ""
• writing the defaultListFormat() helper function and using that in the places you mentioned

We had some final questions:

• we assume the user has the option to configure the ListFormat option through adding it in a yaml file, given that we provided yaml tags next to the struct fields. Is this enough for the user or should we provide another option for configuration?

• we also think it's a good idea to have validation checking for the ListFormat that the user configures through yaml. Where would be the right place to do this validation? Also, is this the full set of valid list formats: csv, json, table, yaml, compact?

• we also noticed the top command only allows for the user to use the table and compact list formats. If we do the validation mentioned above, would we need an extra safe guard for this command?

[stgraber]

• we assume the user has the option to configure the ListFormat option through adding it in a yaml file, given that we provided yaml tags next to the struct fields. Is this enough for the user or should we provide another option for configuration?

That's fine for now. I think we'll probably want to add a command to configure it down the line but we don't need to do it now.

• we also think it's a good idea to have validation checking for the ListFormat that the user configures through yaml. Where would be the right place to do this validation? Also, is this the full set of valid list formats: csv, json, table, yaml, compact?

Those are indeed the valid values, though note that extra options can be passed, like compact,noheader. So when validating, you're going to want to split on , and only validate whatever is passed prior to that.

I'm also not sure that validation is really needed as this turns into the default value for flagFormat which in turn gets validated by the table rendering function. So a user setting a bad value will get an error about it as soon as they try to run any list command which should make it obvious enough that the configuration they just wrote is invalid.

• we also noticed the top command only allows for the user to use the table and compact list formats. If we do the validation mentioned above, would we need an extra safe guard for this command?

Yeah, top indeed only supports table and compact as it doesn't make much sense to have a refreshing wall of YAML or JSON output :)

I don't think this is likely to be a problem though as while the user can definitely set the configuration key to yaml, json or csv, very very few are likely to actually do that as the user experience would be absolutely terrible :)

In practice 99% of those using this feature will use it to set it to compact or to pass extra flags like the aforementioned noheader.

So I don't think that extra care needs to be taken for top. The command will fail if the user sets ListFormat to something other than table or compact, but they can always get passed that error by calling incus top -ftable to override it to a valid value.

[arojas2003]

That makes a lot of sense; thank you! It does seem that the cleaner approach is to rely on validation by the table rendering function.

We will go ahead and make a pull request with our changes.

Once again, thank you for your help!

[harridu]

Thank you. I highly appreciate this improvement.