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

[Zachary Turner via cfe-commits]

zturner added a comment.

In http://reviews.llvm.org/D13549#268701, @curdeius wrote:

> Hi Zachary, just to answer your comments. I have done it on purpose not to use enum, because clang-format style can be actually a JSON string, e.g. `{BasedOnStyle: "LLVM", IndentWidth: 4}`, so it wouldn't translate into an enum (to my knowledge at least). Besides, I would like to have the possibility **not** to add a new enum value if there is a new style in clang-format.
>  Please correct me if I'm wrong about enums.


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).

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.


http://reviews.llvm.org/D13549