WIP : Support Tabular Data formatting in Buildifier
What?
This change adds to the Buildifier the ability to format tabular data using a set of new tags as follows :
Format tabular data in proper visual alignment
#buildifier: format table
Sort tabular data (by column) alongside the allignment
#buildifier: format table sort <column-num>
Why?
The need for the change can be seen with the following example : Use an input file :
tableEx = [
("col1", "col2", "col3"),
("col1111", "col222", "col3333")
]
Run it through buildifier to see the following output :
tableEx = [
("col1","col2","col3"),
("col1111","col222","col3333")
]
Detailing the problem statement :
- Tabular Data can be either "List of Tuples"/ "List of Lists". (See ListExpr , TupleExpr)
- The buildifier's parser stage freezes how an
Exprwill be printed using flags fileForceCompactionfor Tuples , andForceMultilinefor Lists. - These fixed notions of handling the print logic make it difficult to operate with tabular data as shown in the above example.
- Even if the user formats the table himself, the buildfier cannot be indicated "#buildifer : Don't mess with my table". It ruins the user-provided format too. #leaveAlone does not work either.
- We came across this problem when we were building large RPMs with 200 endstates, each having a location of install/permissions, etc as table data.
How?
Design Description :
- Add support for 2 new tags to be parsed with
rewrite.go:#buildifier: format table#buildifier: format table sort <column-num> - Existing architecture of buildifier allows rewriting the AST within
rewrite.gobased on parsed buildifier tags. - Utilize the same pipeline to "mark" the nodes to form the TableRoot and the TableRows based on the presence of the new tags.
- Within
print.go, flex the logic to allow printing a tuple /List in a tabular form when they are identified a TableRoot/TableRow node.
Printing of Tabular data :
- We choose an existing golang package called
TabWriterto jump-start us into this feature without writing custom logic to format rows/column sizes out the output. -
TabWriterimplements "Elastic Tabstops algorithm "
Other approaches evaluated :
- We also evaluated an approach of creating a new Expr class called
TableListExpr,TableRowExpr- It did not provide any advantage over the existing design given the current problem. We can revisit this later if the use cases demand it.
Sorting of Tabular data :
-
Rewrite.gologic where we detect the new tags also sort the row entries based on the index provided in the tag.
Future implementations:
- This changeset only supports a "List of Tuples" as a table. A similar extension can be made for "List of List" as well.
Testing?
- 15 testcases added to support the use-cases
- bazel test ... : PASSED.
Any other info?
There are a couple of known drawbacks due to the TabWriter behavior :
- Especially in the case of "Before Comments" in tableRows. We will be curating them into a document for reviewers soon.
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.
ℹ️ Googlers: Go here for more info.
@googlebot I fixed it.
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.
ℹ️ Googlers: Go here for more info.
@googlebot I fixed it.
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.
ℹ️ Googlers: Go here for more info.
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.
ℹ️ Googlers: Go here for more info.
@googlebot I fixed it.
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.
ℹ️ Googlers: Go here for more info.
@googlebot I fixed it.
@SKashyap looks like a really nice improvement! What's the status of this?
Hey Simon,
Glad to hear that this improvement has caught your interest. This has been in WIP for too long. I am wanting to wrap a couple of golang nits in this change and submit soon.
From: Simon Björklén @.> Reply-To: bazelbuild/buildtools @.> Date: Wednesday, February 2, 2022 at 1:29 AM To: bazelbuild/buildtools @.> Cc: Shwetha Kashyap @.>, Mention @.***> Subject: Re: [bazelbuild/buildtools] WIP : Support Tabular Data formatting in Buildifier (#985)
@SKashyaphttps://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FSKashyap&data=04%7C01%7Ckshwetha%40vmware.com%7C4e58386848b844e8653908d9e62e70c1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637793909414101677%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=NZx%2BtYbIIt9J3tfi9r6cYVNLLaKN1FyQSUnk8lKgiVc%3D&reserved=0 looks like a really nice improvement! What's the status of this?
— Reply to this email directly, view it on GitHubhttps://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fbazelbuild%2Fbuildtools%2Fpull%2F985%23issuecomment-1027741805&data=04%7C01%7Ckshwetha%40vmware.com%7C4e58386848b844e8653908d9e62e70c1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637793909414101677%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=7Dp0dWxvUhSBs8NFykWRuXbzGfWkSawXh1LCd%2Fl4Go4%3D&reserved=0, or unsubscribehttps://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAYIC4FKDQIN4GITVK5H753UZD2NTANCNFSM47EZLGAQ&data=04%7C01%7Ckshwetha%40vmware.com%7C4e58386848b844e8653908d9e62e70c1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637793909414101677%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=NM%2BZOT%2FtRDxy0y3fc8AA%2FnoA8JHl8HfEh%2Fj89MnXvfI%3D&reserved=0. Triage notifications on the go with GitHub Mobile for iOShttps://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fapps.apple.com%2Fapp%2Fapple-store%2Fid1477376905%3Fct%3Dnotification-email%26mt%3D8%26pt%3D524675&data=04%7C01%7Ckshwetha%40vmware.com%7C4e58386848b844e8653908d9e62e70c1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637793909414101677%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=aRyzJjaIPOzTlk0UkosQk%2Bqna4gSKCMXplZB8SlETSg%3D&reserved=0 or Androidhttps://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fplay.google.com%2Fstore%2Fapps%2Fdetails%3Fid%3Dcom.github.android%26referrer%3Dutm_campaign%253Dnotification-email%2526utm_medium%253Demail%2526utm_source%253Dgithub&data=04%7C01%7Ckshwetha%40vmware.com%7C4e58386848b844e8653908d9e62e70c1%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637793909414101677%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=pEOl0Hl2ofTTaBIGJKFJSdcEKV5FiX58AyfxUsVbQfs%3D&reserved=0. You are receiving this because you were mentioned.Message ID: @.***>
@SKashyap have you given up on PR ?
@SKashyap have you given up on PR ?
I had stopped working on this for a while. But have made some recent changes to this in our internal repository. Will collate those and wrap this asap.