[ros2interface] shows empty comments even with --no-comments

[mintar]

Bug report

Required Info:

• Operating System:
  • Ubuntu 20.04
• Installation type:
  • binaries
• Version or commit hash:
  • 0.13.4 (galactic)
• DDS implementation:
  • N/A
• Client library (if applicable):
  • N/A

Steps to reproduce issue

ros2 interface show tf2_msgs/msg/TFMessage --no-comments

Actually, the --no-comments param is not necessary, because it's the default behavior. Just included here for clarification.

Expected behavior

ros2interface should strip all comments:

$ ros2 interface show tf2_msgs/msg/TFMessage --no-comments
geometry_msgs/TransformStamped[] transforms
	std_msgs/Header header
		builtin_interfaces/Time stamp
			int32 sec
			uint32 nanosec
		string frame_id
	string child_frame_id
	Transform transform
		Vector3 translation
			float64 x
			float64 y
			float64 z
		Quaternion rotation
			float64 x 0
			float64 y 0
			float64 z 0
			float64 w 1

Actual behavior

ros2interface does not strip empty comment lines (see the two lines starting with #). This is because of the empty comment lines in tf2_msgs/msg/TFMessage:

$ ros2 interface show tf2_msgs/msg/TFMessage --no-comments
geometry_msgs/TransformStamped[] transforms
	#
	#
	std_msgs/Header header
		builtin_interfaces/Time stamp
			int32 sec
			uint32 nanosec
		string frame_id
	string child_frame_id
	Transform transform
		Vector3 translation
			float64 x
			float64 y
			float64 z
		Quaternion rotation
			float64 x 0
			float64 y 0
			float64 z 0
			float64 w 1

Additional information

[mintar]

The bug isn't just for empty comments, but seems to be any comments that are weirdly indented:

$ ros2 interface show actionlib_msgs/msg/GoalStatusArray --no-comments
std_msgs/Header header
	builtin_interfaces/Time stamp
		int32 sec
		uint32 nanosec
	string frame_id
GoalStatus[] status_list
	GoalID goal_id
		builtin_interfaces/Time stamp
			int32 sec
			uint32 nanosec
		string id
	uint8 status
	uint8 PENDING         = 0
	uint8 ACTIVE          = 1
	uint8 PREEMPTED       = 2
	                            #   and has since completed its execution (Terminal State).
	uint8 SUCCEEDED       = 3
	                            #   (Terminal State).
	uint8 ABORTED         = 4
	                            #    to some failure (Terminal State).
	uint8 REJECTED        = 5
	                            #    because the goal was unattainable or invalid (Terminal State).
	uint8 PREEMPTING      = 6
	                            #    and has not yet completed execution.
	uint8 RECALLING       = 7
	                            #    the action server has not yet confirmed that the goal is canceled.
	uint8 RECALLED        = 8
	                            #    and was successfully cancelled (Terminal State).
	uint8 LOST            = 9
	                            #    be sent over the wire by an action server.
	string text

[clalancette]

Yeah. I spent a bit of time looking at this, and the problem is really down in https://github.com/ros2/rosidl/blob/07c292048a5706d165ef4a75c253759d0ce0c308/rosidl_adapter/rosidl_adapter/parser.py#L465 .

In particular, the code to show an interface is calling parse_message_string against each line of the message definition looked up. This is kind of weird on its own; parse_message_string is usually used to parse entire messages, not line-by-line.

During parse_message_string of a string like #\n, it calls extra_comments, which successfully registers that as a empty string comment (the only case I looked at). However, before it ends up getting put into the msg_spec.annotations['comment'] list, a pass is done to remove leading and trailing whitespace (this is done to remove unnecessary whitespace on block comments when looking at a whole message).

So there are 2 different things to pursue to fix it:

1. Maybe change away from calling parse_message_string for each line, and instead do it for the whole message. I'm not sure how well that would work, but it seems more like what parse_message_string was designed for.
2. Maybe fix up parse_message_string to not strip away empty comments. However, this would need to be thought about and tested thoroughly; I don't know what downstream effects that would have.

All of that said, I probably don't have more time to look at this in the near-term, so I'm going to put it with a help-wanted tag for now.

[fujitatomoya]

However, before it ends up getting put into the msg_spec.annotations['comment'] list, a pass is done to remove leading and trailing whitespace (this is done to remove unnecessary whitespace on block comments when looking at a whole message).

right, currently it cannot tell if comment line or comment is empty instead of adding another annotation, i think.