[PATCH] D13549: Added new options to ClangFormat VSIX package.

Marek Kurdej via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 16 09:07:27 PDT 2015


curdeius added a comment.

> Where is the code in the CL that handles extracting that value from a JSON string?  Because it looks like you're just building an array list of the trivial non-JSON style names, so why couldn't that be an enum?  I don't know mucha bout clang-format, but looking at the comments it seems like the JSON only comes into play when you're reading a .clang-format file, and in that case the value of the enum would be `Style.File` (until you finish reading that, at which case you set it to `Style.LLVM` etc).


The Style option is actually passed to clang-format -style parameter and it's clang-format that parses it. It can be a predefined type (one of: 'none', 'file', 'LLVM', etc.) or a JSON (or JSON-like?) string.

> I'm also not sure what advantage you get by not having to add an enum if there is a new style?  You have to change the code either way, and by not using an enum you are likely to miss some of the places in the code that manipulate or process the style since you're bypassing the type system.


I wanted to say that it would be better if the user didn't have to wait until the VSIX plugins gets updated after a new style is added to clang-format.


http://reviews.llvm.org/D13549





More information about the cfe-commits mailing list