[PATCH] D133757: [clangd] Perform system include extraction inside CommandMangler

Tue Nov 8 14:53:05 PST 2022

ayzhao added a comment.

In D133757#3916040 <https://reviews.llvm.org/D133757#3916040>, @sammccall wrote:

> In D133757#3916028 <https://reviews.llvm.org/D133757#3916028>, @nridge wrote:
>
>> In D133757#3915995 <https://reviews.llvm.org/D133757#3915995>, @ayzhao wrote:
>>
>>> This change is causing system-include-extractor.test <https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/test/system-include-extractor.test> to fail on Chrome: https://crbug.com/1382508
>>>
>>> The test fails on this assertion <https://github.com/llvm/llvm-project/blob/1a2bc103bba04e8fba574c70c6201ba18ccc08ba/clang-tools-extra/clangd/test/system-include-extractor.test#L66> if the path to the LLVM repository contains the plus sign (+). The actual output replaces the plus sign with the URL escape code %2B, but the still expects the plus sign in the output.
>>
>> Thanks for the report, I sent out D137674 <https://reviews.llvm.org/D137674> to work around this.
>
> Oops, I landed a slightly worse version in 96d771c4707df9b3153ea5a8451f2bbbe17c8917 <https://reviews.llvm.org/rG96d771c4707df9b3153ea5a8451f2bbbe17c8917> before seeing your message. @ayzhao it should be fixed at head.

Thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133757