krasimir added a comment.
Just some thoughts.
I agree that `Style.isJavaScript()` is nicer than `Style.Language ==
FormatStyle::LK_JavaScript`, but overall the tradeoffs about adding convenience
methods are not clear to me.
C++/ObjC/ObjC++ is special -- because ObjC++ exists I think it is useful for
isCpp() by default to include LK_ObjC so C++ enhancements added to the
formatter automatically also apply to ObjC++. This is also why the overall
Clang language enum choice makes sense to me. I think we don't want to give an
easy way for people to special case C++ excluding ObjC++. I agree that this
makes `Style.isCpp()` confusing.
Proto/TextProto is another family that occurs frequently together (but much
much less often than C/C++) (as the options format of .proto files is the text
proto format).
I'd be inclined to only add convenience methods for the subsets of languages
that we think are important together and where //improvements to one member of
the subset are most likely to benefit or introduce regressions in other members
of the subset//. I'd be confused if two such convenience methods shared a
language in common.
Specifically, let's provide **exactly** these convenience methods:
likeCpp { LK_Cpp || LK_ObjC }
isCSharp { LK_CSharp }
isJava { LK_Java }
isJavaScript { LK_JavaScript }
isTableGen { LK_TableGen }
likeProto { LK_Proto || LK_TextProto }
and **intentionally not** provide: `isC`, `isCpp`, `isCppOrObjC`, `isObjC`,
`isObjCpp`, `isProto` and `isTextProto`. In cases where those need to be
distinguished, which I believe should be rare enough*, we can use
`Style.Language` comparisons, somewhat documenting that what follows is
non-standard, but also being very precise in each of those situations what
exactly we mean by spelling out the comparisons in full.
The guideline for usages would be: use the convenience methods unless you have
a good reason not to.
//*I realize that probably the most comparisons left would be of the form
`Style.Language == FormatStyle::LK_ObjC` (or `!=`), and that makes me sad, and
maybe this is a good enough reason to add `isObjC` by itself, but then we would
lose the orthogonality of the convenience methods, and we open the door for
adding isXXX for every language kind, which will make it easier to add a
feature for one language that ideally should automatically work by default also
for another, so it sucks either way.//
Sidenote. For "proto" vs "protobuf" -- I don't know whether this language has
an official name. "protobuf" could refer to "the whole ecosystem" vs. "the
definition language (describing types)" vs "a message in binary or text
format". But `.proto` is *the* file extension for the definition language, so I
think that's good enough.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80079/new/
https://reviews.llvm.org/D80079
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
