mprobst added inline comments.

================
Comment at: clang/lib/Format/TokenAnnotator.cpp:3524
       return false;
     if (Left.is(TT_JsTypeOperator) || Right.is(TT_JsTypeOperator))
       return false;
----------------
MyDeveloperDay wrote:
> mprobst wrote:
> > shouldn't you change this line here?
> You know I thought the same and this was where I first put the code change 
> in, but this code doesn't seem to have any effect and I've been caught out by 
> this before... (anyone else seen that?)
> 
> I'm not sure if something has been changed, but I'm finding that often I add 
> code into spaceRequiredBetween() and despite it being executed correctly it 
> doesn't have the desired effect, which is why the code is in 
> spaceRequiredBefore()
Generally we put space between two operators. So this line must have some 
effect, as otherwise we'd always emit `A | B`. Given that, I think we need more 
debugging here to make this change - working around by some more code somewhere 
else doesn't seem like a good long term approach.

Also, note that this is all in `spaceRequiredBefore`, right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117197/new/

https://reviews.llvm.org/D117197

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to