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

Fix table output with wide columns #1191

Conversation

bryfox
Copy link
Contributor

@bryfox bryfox commented Jul 5, 2024

Changelog

Fix: table output from list commands supports wide columns such as large amounts of metadata

Docs

None

Description

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.

BeforeAfter

mcap list metadata would silently fail if a record had >64kb

all metadata is output as expected

As a side effect, this also fixes the column alignment for something like list schemas that contains linebreaks:

BeforeAfter
id  name                encoding  data
24  tf2_msgs/TFMessage  ros1msg   geometry_msgs/TransformStamped[] transforms

================================================================================
MSG: geometry_msgs/TransformStamped
# This expresses a transform from coordinate frame header.frame_id
# to the coordinate frame child_frame_id
#
# This message is mostly used by the
# <a href="http://wiki.ros.org/tf">tf</a> package.
# See its documentation for more information.

Header header
string child_frame_id # the frame id of the child frame
Transform transform

id	name              	encoding	data
24	tf2_msgs/TFMessage	ros1msg 	geometry_msgs/TransformStamped[] transforms

  	                  	        	================================================================================
  	                  	        	MSG: geometry_msgs/TransformStamped
  	                  	        	# This expresses a transform from coordinate frame header.frame_id
  	                  	        	# to the coordinate frame child_frame_id
  	                  	        	#
  	                  	        	# This message is mostly used by the
  	                  	        	# <a href="http://wiki.ros.org/tf">tf</a> package.
  	                  	        	# See its documentation for more information.

  	                  	        	Header header
  	                  	        	string child_frame_id # the frame id of the child frame
  	                  	        	Transform transform

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

Fixes #1189.

Copy link
Collaborator

@james-rms james-rms left a comment

Choose a reason for hiding this comment

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

This change is good. It's not clear to me why other commands need to use the original formatter rather than this one - is there a regression, or did you not want to change too many things in this PR?

@bryfox
Copy link
Contributor Author

bryfox commented Jul 9, 2024

It's not clear to me why other commands need to use the original formatter rather than this one

My goal was to leave the output of mcap info exactly as-is. I was not able to get correct formatting for both 'info' and other commands using the same formatter. They're really different formats. Most are column-aligned tables; info has left-aligned labels with tabs in the row content itself to indent some rows.

@bryfox bryfox merged commit 95e2b44 into main Jul 9, 2024
24 checks passed
@bryfox bryfox deleted the bryan/fg-8036-cli-mcap-list-metadata-doesnt-show-the-metadata-table-with branch July 9, 2024 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

[CLI] mcap list metadata doesn't show the metadata table with huge amount of data in a single entry
2 participants