[PATCH] D93528: [clang-format] Add basic support for formatting JSON

Marek Kurdej via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 18 07:18:22 PST 2020


curdeius added a comment.

In D93528#2463030 <https://reviews.llvm.org/D93528#2463030>, @HazardyKnusperkeks wrote:

> In D93528#2462969 <https://reviews.llvm.org/D93528#2462969>, @MyDeveloperDay wrote:
>
>> So I guess my question is:
>>
>> 1. is this useful to pursue (either as is, or by switching to us the reformat method)
>> 2. is this useful standalone as the first pass
>> 3. is adding support for JSON something people would welcome (as we did with C#)
>>
>> If the answer is no, then I don't want to waste the effort trying to fathom how we might do this inside reformat.
>
> For me that's three times yes.

For me as well. I believe it's a useful addition, even if it handles only the basic cases for the moment.
I was just wondering whether it would be better to start a different way, but I understand that implementing it the "clang-format's way" can take some effort.

Anyway, I'd like to see a big warning banner (somewhere in the doc) about current limitations, so that nobody switches hastily to clang-format and then gets disappointed by these limitations.
Not sure that having a small note about it in release notes is enough.
So to the point, please document that only full-context/file is supported and that the only formatting option is indentation level (as you partially did in release notes).



================
Comment at: clang/docs/ReleaseNotes.rst:284
 
+- Basic Support has been adding for Formatting .json files (with very limited options)
+
----------------
Maybe instead of putting "with very limited options", you may add a link to the doc describing limitations?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93528/new/

https://reviews.llvm.org/D93528



More information about the cfe-commits mailing list