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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
