jack2012aa commented on PR #20536: URL: https://github.com/apache/kafka/pull/20536#issuecomment-3344756845
> > > This is a nice improvement but I wonder whether it could be even better with a bit of tweak. Here's an example of the current output. > > > The part that could do with improvement is the table entries `__tagged_fields The tagged fields` rows in the table. It's all so nice now, apart from that. > > > > > > @AndrewJSchofield Thanks for the suggestion. Since the ticket didn't mention the table, I have few question on how to modify it. > > 1.Are there reasons why empty tagged fields are left in the table? Removing them is easy, but I want to make sure it is appropriate. 2. Should we remove the nested table of tagged fields, to make the format consist to the BNF? > > 1. My view is that the empty `__tagged_fields` entries are redundant. Now that we are properly rendering the tagged fields, I would remove the empty ones. > 2. Provided that it's all comprehensible when compared with the BNF, I think that would be fine. The new output is included in the description. Thank you! Besides the current changes, I found an inconsistency in how nested fields are displayed: * Normal Fields: Their inner fields are flattened into the main table. * Tagged Fields: Their inner fields are nested within a separate, wrapped table. I think unifying this would improve readability. My recommendation is to use the flattened style for all fields (including tagged ones), as the parent-child relationship is already clear from the BNF format next to it. This would make the entire table structure cleaner and more consistent. What do you think of this approach? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
