daiyousei-qz added inline comments.
================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:578
+ constexpr unsigned HintMinLineLimit = 2;
+ constexpr unsigned HintMaxLengthLimit = 50;
+
----------------
nridge wrote:
> daiyousei-qz wrote:
> > sammccall wrote:
> > > We actually already have a configurable inlay hint length limit.
> > > It has the wrong name (`TypeNameLimit`) , but I think we should use it
> > > anyway (and maybe try to fix the name later).
> > >
> > > (I'm not sure making this configurable was a great idea, but given that
> > > it exists, people will expect it to effectively limit the length of hints)
> > `TypeNameLimit` was introduced to limit the length of "DeducedTypes" only.
> > I don't think using that number for all hints is a good idea because
> > different hints appear in different contexts. For deduced type hint, it is
> > displayed in the middle of the actual code, so it must be concise to not
> > overwhelm the actual code. However, this hint is usually displayed at the
> > end of an almost empty line. A much larger number of characters should be
> > allowed.
> >
> > (I'm not arguing here, but IMO such options are definitely helpful. Not
> > everybody would fork llvm-project and build their own version of clangd
> > binary. Without options to configure length of hints, people would disable
> > "DeducedTypes" entirely if they cannot tolerate the default length limit,
> > while this feature is definitely a cool thing to turn on. Personally, I
> > actually think clangd has too little option compared to say rust-analyzer.
> > But it's just my understanding)
> > For deduced type hint, it is displayed in the middle of the actual code, so
> > it must be concise to not overwhelm the actual code. However, this hint is
> > usually displayed at the end of an almost empty line. A much larger number
> > of characters should be allowed.
>
> Another consideration besides the location of the hint that is worth keeping
> in mind, is what type of content is being printed.
>
> Type names in C++ are composable in ways that identifiers are not (e.g.
> `vector<basic_string<char, char_traits<char>, allocator<char>,
> allocator<basic_string<...>>` etc.), such that there is a need to limit type
> hints that doesn't really apply to say, parameter hints.
>
> So, a question I have here is: can template argument lists appear in an
> end-definition-comment hint?
>
> For example, for:
>
> ```
> template <typename T>
> struct S {
> void foo();
> };
>
> template <typename T>
> void S::foo() {
> } // <--- HERE
> ```
>
> is the hint at the indicated location `S::foo()`, or `S<T>::foo()`? In the
> latter case, we can imagine the hint getting long due to the type parameters,
> and we may want to consider either limiting its length, or tweaking the code
> to not print the type parameters.
Yes, currently this hint is only shown if the total length of hint label is
less than 60 characters. I believe what we display should be exactly what is
used to define the symbol. In your example,
```
template <typename T>
struct S {
void foo();
};
template <typename T>
void S<T>::foo() {
} // S<T>::foo
```
Basically, "[A]" and "[B]" is the same text (OFC excluding whitespaces).
```
void [A]() {
} // [B]
```
The hint won't be displayed if "[B]" is more than 60 characters.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D150635/new/
https://reviews.llvm.org/D150635
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
