Skip to content

Commit

Permalink
Fix table output with wide columns (#1191)
Browse files Browse the repository at this point in the history
Fix: table output from list commands supports wide columns such as large
amounts of metadata

Previously, listing the metadata for an mcap file with many (>64kb)
key/value pairs failed silently.

The Scanner used in the table formatter has a buffer limit of 64kb, and
the error result was being ignored. This removes the scanner entirely
from most table output. It was introduced to trim leading whitespace
from lines, but the formatter accepts other options to make this happen.

I've given the info command its own formatter, which I think is
appropriate because it's formatting data differently, even including
tabs in its row data. The scanner's error result is now checked, though
we shouldn't see this in practice for `mcap info`.

This also makes a couple of copy fixes to the inline help for the
metadata commands.

Fixes #1189.
  • Loading branch information
bryfox authored Jul 9, 2024
1 parent 037ceb7 commit 95e2b44
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 14 deletions.
30 changes: 28 additions & 2 deletions go/cli/mcap/cmd/info.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"bufio"
"bytes"
"context"
"fmt"
Expand All @@ -13,6 +14,7 @@ import (

"github.com/foxglove/mcap/go/cli/mcap/utils"
"github.com/foxglove/mcap/go/mcap"
"github.com/olekukonko/tablewriter"
"github.com/spf13/cobra"
)

Expand Down Expand Up @@ -89,7 +91,9 @@ func printInfo(w io.Writer, info *mcap.Info) error {
header = addRow(header, "end:", "%s", decimalTime(endtime))
}
}
utils.FormatTable(buf, header)
if err := printSummaryRows(buf, header); err != nil {
return err
}
if len(info.ChunkIndexes) > 0 {
compressionFormatStats := make(map[mcap.CompressionFormat]struct {
count int
Expand Down Expand Up @@ -166,7 +170,9 @@ func printInfo(w io.Writer, info *mcap.Info) error {
}
rows = append(rows, row)
}
utils.FormatTable(buf, rows)
if err := printSummaryRows(buf, rows); err != nil {
return err
}
if info.Statistics != nil {
fmt.Fprintf(buf, "attachments: %d\n", info.Statistics.AttachmentCount)
fmt.Fprintf(buf, "metadata: %d\n", info.Statistics.MetadataCount)
Expand All @@ -178,6 +184,26 @@ func printInfo(w io.Writer, info *mcap.Info) error {
return err
}

// Similar to utils.FormatTable, but optimized for 'expanded' display of nested data.
func printSummaryRows(w io.Writer, rows [][]string) error {
buf := &bytes.Buffer{}
tw := tablewriter.NewWriter(buf)
tw.SetBorder(false)
tw.SetAutoWrapText(false)
tw.SetAlignment(tablewriter.ALIGN_LEFT)
tw.SetHeaderAlignment(tablewriter.ALIGN_LEFT)
tw.SetColumnSeparator("")
tw.AppendBulk(rows)
tw.Render()
// This tablewriter puts a leading space on the lines for some reason, so
// remove it.
scanner := bufio.NewScanner(buf)
for scanner.Scan() {
fmt.Fprintln(w, strings.TrimLeft(scanner.Text(), " "))
}
return scanner.Err()
}

var infoCmd = &cobra.Command{
Use: "info",
Short: "Report statistics about an MCAP file",
Expand Down
4 changes: 2 additions & 2 deletions go/cli/mcap/cmd/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ var addMetadataCmd = &cobra.Command{

var getMetadataCmd = &cobra.Command{
Use: "metadata",
Short: "get metadata by name",
Short: "Get metadata by name",
Run: func(_ *cobra.Command, args []string) {
ctx := context.Background()
if len(args) != 1 {
Expand Down Expand Up @@ -214,7 +214,7 @@ func init() {
}

getCmd.AddCommand(getMetadataCmd)
getMetadataCmd.PersistentFlags().StringVarP(&getMetadataName, "name", "n", "", "name of metadata record to create")
getMetadataCmd.PersistentFlags().StringVarP(&getMetadataName, "name", "n", "", "name of metadata record to get")
err = getMetadataCmd.MarkPersistentFlagRequired("name")
if err != nil {
die("failed to mark --name flag as required: %s", err)
Expand Down
14 changes: 4 additions & 10 deletions go/cli/mcap/utils/utils.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
package utils

import (
"bufio"
"bytes"
"context"
"encoding/json"
"fmt"
"io"
"os"
"regexp"
"strings"
"time"

"cloud.google.com/go/storage"
Expand Down Expand Up @@ -107,21 +105,17 @@ func WithReader(ctx context.Context, filename string, f func(remote bool, rs io.
}

func FormatTable(w io.Writer, rows [][]string) {
buf := &bytes.Buffer{}
tw := tablewriter.NewWriter(buf)
tw := tablewriter.NewWriter(w)
tw.SetBorder(false)
tw.SetAutoWrapText(false)
tw.SetAlignment(tablewriter.ALIGN_LEFT)
tw.SetHeaderAlignment(tablewriter.ALIGN_LEFT)
tw.SetColumnSeparator("")
tw.SetTablePadding("\t")
tw.SetNoWhiteSpace(true)

tw.AppendBulk(rows)
tw.Render()
// This tablewriter puts a leading space on the lines for some reason, so
// remove it.
scanner := bufio.NewScanner(buf)
for scanner.Scan() {
fmt.Fprintln(w, strings.TrimLeft(scanner.Text(), " "))
}
}

func Keys[T any](m map[string]T) []string {
Expand Down

0 comments on commit 95e2b44

Please sign in to comment.