Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: text/template: add table action #45752

Closed
heaths opened this issue Apr 24, 2021 · 15 comments
Closed

proposal: text/template: add table action #45752

heaths opened this issue Apr 24, 2021 · 15 comments

Comments

@heaths
Copy link

heaths commented Apr 24, 2021

I'd like to propose adding a new node to text/template similar to {{range pipeline}} named {{table pipeline}}. Inspired by Docker CLI's pseudo-function to format an array using text/tabwriter, this could be a generally useful way to format an array as columnar data using elastic tabstops.

As noted in cli/cli#3488 (comment), Docker CLI checks if the template starts with "table" and does some simple string pre-parsing. The inherent problem is that the template must start with "table", so you can't first scope a pipeline with {{with pipeline}}, for example. You can't really do this wthin a string either because you need to know where the table columns should end, and only parse tree nodes have {{end}}.

I propose (and interested in contributing) something like range that works on an array, but lets you delimit columns like so:

nodes := []struct{
  Nodes []struct{
    Id string
    Name string
  }
  Description string
}{
  // ...
}

template.Must(template.New("main").Parse(`{{table .Nodes}}{{.Id}}{{"\t"}}{{.Name}}{{end}}`)).Execute(os.Stdout, nodes)

This would product output like:

123  Foo
4567 Bar
8    Baz

The {{end}} node would call Writer.Flush which should effectively set the final tabstops.

Ideally, it would be great to provide some control over tabwriter.NewWriter's arguments. Perhaps {{table}} could support variadic ...[]string arguments to set a few options, but columns can help support that on their own, like a user-defined pad function that prefixes or suffixes padding characters like "..", changing colors, etc. Another option might be to emit a header using the same tabstops so the headers align with the columns, much like Docker CLI does with their pseudo-function.

@gopherbot gopherbot added this to the Proposal milestone Apr 24, 2021
@seankhliao seankhliao changed the title Proposal: add table node to text/template proposal: text/template: add table action Apr 24, 2021
@seankhliao seankhliao changed the title proposal: text/template: add table action proposal: text/template: add table function Apr 24, 2021
@seankhliao seankhliao changed the title proposal: text/template: add table function proposal: text/template: add table action Apr 24, 2021
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Apr 24, 2021
@ianlancetaylor
Copy link
Contributor

Could you just feed the output of text/template into text/tabwriter?

@heaths
Copy link
Author

heaths commented Apr 25, 2021

From the perspective of providing useful template functions to users to simply format output without writing or installing another tool, that doesn't seem like a very useful approach. Both the Docker CLI and GitHub CLI (and likely many other tools accepting go templates) provide a templating. Seems making text/tabwriter available via this mechanism would be generally useful to provide tidy output to the console.

@heaths
Copy link
Author

heaths commented Apr 25, 2021

While I realize this proposal as written may not be acceptable because it emits formatted text directly instead of facilitating formatting, perhaps it would be better to consider alternatives that still provide the core formatting to functions.

For instance, if we were to try to do this in a function, something like this could work:

{{range .Nodes}}{{tableRow .Id .Name}}{{end}}

However, we lose any ability to pass fields to other custom functions. Perhaps we could consider something like this where parenthesis are parsed by the text/template engine:

{{range .Nodes}}{{tableRow (.Id | color "green") .Name}}{{end}}

Here, {{.Id}} would be passed to the custom color function before the resulting string is passed to the custom tableRow function.

@heaths
Copy link
Author

heaths commented Apr 25, 2021

I see you can already use parenthesis like that: cli/cli#3488 (comment)

While I'm tempted to just close this proposal out, I would like to still see if there's interest in it. While I have a solution (or workaround at least), I think this could still be useful, though I realize it might be out of scope since it would emit formatted text itself instead of merely facilitating in formatting.

@ianlancetaylor
Copy link
Contributor

I'm not suggesting installing another tool, I'm suggesting something like (untested)

func TemplateTable(tmpl *template.Template, w io.Writer) error {
    var tabw tabwriter.Writer
    tabw.Init(w, ...)
    tmpl.Execute(&tabw, ...)
}

@heaths
Copy link
Author

heaths commented Apr 25, 2021

Ah, I see. That's clever and if a table format was all we wanted to support I agree that's very straight forward. In this case, I'm sure the CLI works want to keep the format open-ended.

@rsc
Copy link
Contributor

rsc commented May 5, 2021

This is a very specific instance of a general request to be able to pipe a block of template output through another function. If we could do that, then "table" could be defined as a function outside the template package. But that's not possible today.

@heaths
Copy link
Author

heaths commented May 5, 2021

I was able to devise a way: cli/cli#3488

Originally I used text/tabwriter, but after feedback switched over to the TablePrinter the GitHub CLI already used internally. I ended up adding just a row function since declaring and ending a table wasn't really necessary. It works with range like so:

{{range .}}{{row (printf "#%v" .id | autocolor "green") .title}}{{end}}

@rsc
Copy link
Contributor

rsc commented May 5, 2021

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc rsc moved this from Incoming to Active in Proposals (old) May 5, 2021
@heaths
Copy link
Author

heaths commented May 5, 2021

I'd love to contribute my work (the original work using tabwriter, perhaps, though some refactorings for performance would be good) if approved. That said, would it be appropriate to pile on a few other related ideas or better as a separate proposal? Specifically, supporting "safe navigation operators" (a proposal to golang itself that was rejected) for templates since you have a limited grammar. For example, if I query for a nested string, there's no way short of duplicating parts of templates between if/else/end to handle the inevitable error. For example, GitHub can send back status { message } as a leaf node, but if status is nil, .status.message errs. The built-in and and or functions evaluate (or, rather, they are evaluated before being passed to those functions) all operands so those don't work either. Instead, perhaps supporting .status?.message would be ideal. The parameter would have to be an interface{}, or perhaps the grammar extended to something like .status?.message || "".

(Was also posted for discussion to https://forum.golangbridge.org/t/support-for-safe-navigation-in-text-templates/23325)

@rsc
Copy link
Contributor

rsc commented May 12, 2021

The bar for adding new things to template is pretty high.
The table concept here is probably not clearing that bar.

/cc @robpike

@robpike
Copy link
Contributor

robpike commented May 12, 2021

I agree that this does not bring enough new power to the library to be worth the change.

@heaths
Copy link
Author

heaths commented May 13, 2021

Understood. I can open a separate issue for the safe navigation operator, but wondering if you think that also wouldn't right to the bar given my use case above. If it's at least worth discussing in a separate issue, I'm happy to open one.

@heaths heaths closed this as completed May 13, 2021
@seankhliao
Copy link
Member

I believe "safe navigation" is #43608

@rsc rsc moved this from Active to Declined in Proposals (old) May 19, 2021
@rsc
Copy link
Contributor

rsc commented May 19, 2021

No change in consensus, so declined.
— rsc for the proposal review group

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

6 participants