Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: add generic table formatter#3415

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
deansheather merged 5 commits intomainfromtable-formatter
Aug 17, 2022
Merged

Conversation

deansheather
Copy link
Member

Adds new functioncliui.DisplayTable(items []T, sortBy string, filterColumns []string) (string, error) which will render a table and return it as a string.

Uses the reflect package to sniff the columns using atable:"name" struct tag. If any of the input types are invalid, this will always panic (even for empty slices!). The input must be a slice or a pointer to a slice that concretely holds a single struct type or a pointer to a struct type. The inner struct type must have at least onetable:"name" tag.,recursive cannot be specified if the field is not a struct or a pointer to a valid tableable struct.

Gist of how it works:

  1. Ensure the input type is a slice or a pointer to a slice (as it's really an interface{} input type to avoid generics clutter in the output binary)
  2. Look at the type that is contained in the slice (even if it's empty or nil) to figure out what columns exist so we can render a header
    1. Ensure it's a struct or a pointer to a struct
    2. Iterate over every field and append to a list the name of the field
    3. If we encounter a field with the tagtable:"name,recursive":
      1. Ensure the field's type is a struct or a pointer to a struct
      2. Repeat step 2 but capture in a separate list
      3. Append to the parent's column list, but append the parent field name from 2.3 like so:"$ParentName $ChildName"
    4. Return a[]string containing the column names for the table
  3. Iterate over every value in the slice and convert tomap[string]interface{}, a map of column names to values
  4. Print the table values by checking for the columns in each value's map, or printing<nil> if the value is not present in the map

@deansheather
Copy link
MemberAuthor

$go run ./cmd/coder users listUSERNAME       EMAIL                              CREATED AT       STATUS...deansheather   dean@************.com              Jun  1 15:28:07  active...

@deansheatherdeansheather requested a review froma team as acode ownerAugust 8, 2022 18:45
Copy link
Member

@Kira-PilotKira-Pilot left a comment

Choose a reason for hiding this comment

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

FE ✅

Copy link
Member

@mafredrimafredri left a comment

Choose a reason for hiding this comment

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

Really nice work! Code looks good, so I'm approving this. I'd still like us to avoidpanicing though.

for i := 0; i < v.Len(); i++ {
// Format the row as a slice.
rowMap := valueToTableMap(v.Index(i))
rowSlice := make([]interface{}, len(headers))
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion (non-blocking): It's a post-generics crazy world, consider:

Suggested change
rowSlice:=make([]interface{},len(headers))
rowSlice:=make([]any,len(headers))

(There are more instances, feel free to reject this suggestion if you don't want to make the changes.)

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I don't mind if you want to change them, but if we care we should really add a linter for this.

Originally this function was gonna accept []T but I decided against it because I didn't want to clutter the binary with duplicated functions

Copy link
Member

Choose a reason for hiding this comment

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

I don't have a huge preference, but I see we have mixed use and it'd be nice to standardize. As you propose, a linter would be a great idea.

tw.AppendHeader(headers)
tw.SetColumnConfigs(FilterTableColumns(headers, filterColumns))
tw.SortBy([]table.SortBy{{
Name: sort,
Copy link
Member

Choose a reason for hiding this comment

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

Should we verify thatsort is a valid column name (e.g. check headers)?

Copy link
Member

@mafredrimafredri left a comment

Choose a reason for hiding this comment

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

New changes look great!:shipit:

@deansheatherdeansheather merged commita872330 intomainAug 17, 2022
@deansheatherdeansheather deleted the table-formatter branchAugust 17, 2022 16:28
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@mafredrimafredrimafredri approved these changes

@kylecarbskylecarbskylecarbs approved these changes

@Kira-PilotKira-PilotKira-Pilot approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@deansheather@mafredri@kylecarbs@Kira-Pilot

[8]ページ先頭

©2009-2025 Movatter.jp