zhangyi1357 added a comment.
I dont have commit access. Could you help committing the change for me? @hokein
================
Comment at: clang-tools-extra/clangd/Config.h:151
+ // Limit the length of type names in inlay hints.
+ size_t TypeNameLimit = 32;
} InlayHints;
----------------
hokein wrote:
> zhangyi1357 wrote:
> > hokein wrote:
> > > I would extend it a bit more -- 0 means no limit.
> > >
> > > Can you also add a unittest in `TEST(TypeHints, LongTypeName)` in
> > > `InlayHintTests.cpp`?
> > > 0 means no limit.
> > This is quite a good idea. I've done it.
> >
> > For unittest, there is already `TEST(TypeHints, LongTypeName)` in
> > `InlayHintTests.cpp`. Do you mean add more tests in the same `TEST` or
> > another `TEST` with TypeNameLimit configured?
> >
> I mean adding one more test in the same `TEST(TypeHints, LongTypeName)`.
>
> This test verifies the the long type name is shown when the limit is set to
> 0, something like
>
> ```
> TEST(TypeHints, LongTypeName) {
> assertTypeHints(R"cpp(
> template <typename, typename, typename>
> struct A {};
> struct MultipleWords {};
> A<MultipleWords, MultipleWords, MultipleWords> foo();
> // Omit type hint past a certain length (currently 32)
> auto var = foo();
> )cpp");
>
> Config cfg;
> ... // set the limit to 0
>
> assertTypeHints(R"cpp(
> template <typename, typename, typename>
> struct A {};
> struct MultipleWords {};
> A<MultipleWords, MultipleWords, MultipleWords> foo();
> // Omit type hint past a certain length (currently 32)
> auto var = foo();
> )cpp", ExpectedHint...);
> }
> ```
Thanks for your example! I've added that.
================
Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:80
C.InlayHints.Designators = false;
+ C.InlayHints.TypeNameLimit = 1;
return C;
----------------
hokein wrote:
> why do we need this change? I think it should be fine without it.
Yes, without this line the tests will not fail. But I am a little confused
about the code below without 'C.InlayHints.TypeNameLimit = 1;'.
```
Config noHintsConfig() {
Config C;
C.InlayHints.Parameters = false;
C.InlayHints.DeducedTypes = false;
C.InlayHints.Designators = false;
// C.InlayHints.TypeNameLimit = 1;
return C;
}
template <typename... ExpectedHints>
void assertHints(InlayHintKind Kind, llvm::StringRef AnnotatedSource,
ExpectedHints... Expected) {
Annotations Source(AnnotatedSource);
TestTU TU = TestTU::withCode(Source.code());
TU.ExtraArgs.push_back("-std=c++20");
auto AST = TU.build();
EXPECT_THAT(hintsOfKind(AST, Kind),
ElementsAre(HintMatcher(Expected, Source)...));
// Sneak in a cross-cutting check that hints are disabled by config.
// We'll hit an assertion failure if addInlayHint still gets called.
WithContextValue WithCfg(Config::Key, noHintsConfig());
EXPECT_THAT(inlayHints(AST, std::nullopt), IsEmpty()); // Why does this
succeed with TypeNameLimit = 32 ?
}
TEST(TypeHints, Smoke) {
assertTypeHints(R"cpp(
auto $waldo[[waldo]] = 42;
)cpp",
ExpectedHint{": int", "waldo"});
}
```
The dault TypeNameLimit is 32, why does the second `EXPECT_THAT` succeed with
TypeNameLimit = 32.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147395/new/
https://reviews.llvm.org/D147395
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
