MyDeveloperDay marked an inline comment as done.
MyDeveloperDay added a comment.
> With my maintainer-of-`Support/JSON` hat on, I don't like the idea of these
> features being shoehorned into the library to make clang-format an
> incrementally more featureful JSON formatter. They're likely to lead to a lot
> of conceptual+implementation+API complexity in a library that solves its
> primary use cases quite well and simply.
No plans at all to touch Support/JSON, if this needed a separate reformat() for
json then I'd say I'm more likely to do that in lib/Format as we'd only need a
"PrettyPrint" type function which supported the options we want to support
> So if the plan is to
>
> - use this implementation to bootstrap JSON-in-clang-format
That was kind of the plan
> - with a firm intention of replacing it with parsing/formatting logic that
> lives inside clang-format itself
So I tried again to do this yesterday, but once again seems to fail, mainly
around the area of clang-formatting trying to reflow and rejoin split lines but
also the level of indentation is off.
I'm starting to wonder if perhaps it needs its own separate pass, use more of
clang-format but less of the actual "parsing into known structures"...
> - long-term, maybe making use of `Support/JSON` long-term for some low level
> bits (normalizing string tokens?) but not where flexibility is needed
I think I'd really just like to use the `FormatTokens`, but I agree if we ended
up writing a specific JSON pretty-printer I could see using the inner parts
(but I really have no interesting in touching lib/Support/JSON at all)
> then I think this is great. However I'm a bit leery if the plan is "land this
> a MVP, and then play it by ear".
Sorry I'm not sure I understand what MVP means in this context?
================
Comment at: clang/unittests/Format/FormatTestJson.cpp:14
+
+#define DEBUG_TYPE "format-test"
+
----------------
HazardyKnusperkeks wrote:
> I don't know what's the practice right now. But I would suggest renaming that
> to "format-test-json", so you can see directly that's for JSON.
I'm not sure of the convention or even where/when its used (I just know you
need it)
lets take that change offline, we should either change them all or none of the
them.
```
FormatTest.cpp:#define DEBUG_TYPE "format-test"
FormatTestCSharp.cpp:#define DEBUG_TYPE "format-test"
FormatTestComments.cpp:#define DEBUG_TYPE "format-test"
FormatTestJS.cpp:#define DEBUG_TYPE "format-test"
FormatTestJava.cpp:#define DEBUG_TYPE "format-test"
FormatTestJson.cpp:#define DEBUG_TYPE "format-test"
FormatTestObjC.cpp:#define DEBUG_TYPE "format-test"
FormatTestProto.cpp:#define DEBUG_TYPE "format-test"
FormatTestRawStrings.cpp:#define DEBUG_TYPE "format-test"
FormatTestSelective.cpp:#define DEBUG_TYPE "format-test"
FormatTestTableGen.cpp:#define DEBUG_TYPE "format-test"
FormatTestTextProto.cpp:#define DEBUG_TYPE "format-test"
NamespaceEndCommentsFixerTest.cpp:#define DEBUG_TYPE
"namespace-end-comments-fixer-test"
SortImportsTestJS.cpp:#define DEBUG_TYPE "format-test"
SortImportsTestJava.cpp:#define DEBUG_TYPE "format-test"
SortIncludesTest.cpp:#define DEBUG_TYPE "format-test"
UsingDeclarationsSorterTest.cpp:#define DEBUG_TYPE
"using-declarations-sorter-test"
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93528/new/
https://reviews.llvm.org/D93528
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
