mprobst marked 2 inline comments as done.
================
Comment at: lib/Format/TokenAnnotator.cpp:155
@@ +154,3 @@
+ } else if (Style.Language == FormatStyle::LK_JavaScript && Left->Previous
&&
+ Left->Previous->is(TT_JsTypeColon)) {
+ // let x: (SomeType);
----------------
djasper wrote:
> I'd merge this one in with the previous.. And maybe even the one from above
> so that we end up with:
>
> } else if (Style.Language == FormatStyle::LK_JavaScript && Left->Previous
> &&
> (Line.First->is(Keywords.kw_type) ||
> Left->Previous->isOneOf(Keywords.kw_function, TT_JsTypeColon)
> ||
> (Left->Previous->endsSequence(tok::identifier,
> Keywords.kw_function)))) {
>
> (in order for Line.First to be "type", Left->Previous cannot be nullptr, so
> this should be equivalent)
I think I'd prefer keeping this as is. The way it is now formulated is more
verbose, but that's to communicate and keep apart three very different
situations (type alias, (async)? function, variable declaration).
Merging the boolean conditions in some clever way will make it very hard to
reverse-decipher which partially overlapping boolean clause is for which of
these situations, and it's usually non-obvious if test coverage is good enough
to make sure nothing breaks if you refactor.
The could would also be harder to read, e.g. the is(kw_function) part is
logically related to the endsSeqeunce call, but they end up in different parts
of the clause.
So, in the interest of readability and maintainability, I'll keep it like this,
and assume by accepting the diff you're cool with that :-) If not, please do
push back.
================
Comment at: lib/Format/TokenAnnotator.cpp:926
@@ +925,3 @@
+ // Type aliases use `type X = ...;` in TypeScript.
+ !(Style.Language == FormatStyle::LK_JavaScript &&
+ Line.First->is(Keywords.kw_type)) &&
----------------
djasper wrote:
> I'd move the ! into the parentheses, but doesn't matter much.
I've formulated it like this intentionally – it follows my brain waves, as in
"mark this as an expression if not this is javascript and it's a type def".
Changing it to "mark this as an expression if language is not javascript or
line is not a typedef" is less readable to me.
http://reviews.llvm.org/D21597
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
