[PATCH] D80079: [clang-format] [NFC] isCpp() is inconsistently used to mean both C++ and Objective C, add language specific isXXX() functions

[Krasimir Georgiev via Phabricator via cfe-commits]

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