Tweak the hover popup information to make it more readable and concise

[somedude72]

Feature Request

Hi!

The current hover popup that cpptools show seems to expand every namespace for the symbol under the cursor. While this approach does eliminate any ambiguities in terms of where the symbol comes from (e.g. whether it comes from the stl or external libraries), it sometimes can destroy the readability of the hover popups. This is demonstrated below in a few screenshots:

As demonstrated above, the fully expanded namespaces of most standard library classes, functions, and methods are needlessly long, making reading even the most trivial stl container types (such as "std::__1::vector<std::__1::string>") cause unnecessary overhead.

In contrast, clangd's hover popup shows the declaration pulled directly from the original file, with only template arguments filled in and macros expanded. This approach, while creating more ambiguity as to where the symbol under the cursor came from, makes for a much clearer and cleaner presentation and eliminates the unnecessary overhead from reading a long string of "std::__1"s. This is demonstrated below in a few screenshots:

My feature request is to add more configuration for the user to customize how the hover definition should be shown. It would be nice to specify to cpptools whether to expand only namespaces, only macros, only template arguments, or any combination of it. This would both satisfy situations where users might require more specificity in terms of where the symbol came from, and situations where value cleaner hover popups is desired.

Thanks!

[bobbrow]

For the MSVC compiler on Windows and g++ on linux it looks like we are applying the type aliases and don't have this problem. I do see the behavior you describe on macOS with clang though. Is that what you're using?

@SomedudeX would you consider it sufficient to remove the ::__1 from these types? I'm not sure that we'd want to show string instead of std::string (especially in cases where using namespace std; is not in effect). And it's not clear to me what makes clangd pick one over the other since it does both.

For your three examples, it would then look more like the g++ versions:

• inline std::ostream &std::ostream::operator<<(std::ostream &(*__pf)(std::ostream &))
• inline void std::vector<std::string>::push_back(std::string &&__x)
• std::vector<std::string> v

[somedude72]

For the MSVC compiler on Windows and g++ on linux it looks like we are applying the type aliases and don't have this problem. I do see the behavior you describe on macOS with clang though. Is that what you're using?

I am indeed using macOS with llvm/clang.

I'm not sure that we'd want to show string instead of std::string (especially in cases where using namespace std; is not in effect). And it's not clear to me what makes clangd pick one over the other since it does both.

After some experimenting, I think the way that clangd decides whether to include namespace specifiers in the hover is by looking at the in definition of the symbol the original file. So for the std::vector<std::string> v example, clangd included the std:: because the definition in the file contained the std:: specifier. Whereas for the std::vector::push_back example, it didn't include std:: because the push_back definition in llvm's libc++ vector header file didn't have the std:: specifier.

@SomedudeX would you consider it sufficient to remove the ::__1 from these types?

I think it would be sufficient to remove the ::__1s.

And out of curiosity, is expanding ::__1 currently an intended behavior or is it a bug with cpptools on macOS clang?

[bobbrow]

And out of curiosity, is expanding ::__1 currently an intended behavior or is it a bug with cpptools on macOS clang?

Technically, I believe the answer is that this is expected behavior, but let's explore the case a bit. I'm open to changing the behavior because in this case the behavior you want makes total sense. This is what we're dealing with:

In the libcpp headers that clang uses, __1 is an inline namespace and it defines basic_string in it. It also typedef's string within this namespace. g++'s headers also use an inline namespace __cxx11 to define basic_string, but when it typedef's string it does it outside of the inline namespace. What we report for the tooltip is based on the type alias and in clang's case they defined it in std::__1.

We could, in theory, always eliminate inline namespaces from the tooltips because the result will be correct and will also be what users of the standard library expect to see, but what if the developer uses this feature for versioning their own types? Will they expect to see the inline namespace for their types?

namespace example {
    inline namespace v2 {
        struct foo {
            int n2;
        };
    }

    namespace v1 {
        struct foo {
            int n1;
        };
    }
}

void test() {
    example::v1::foo f1;
    example::v2::foo f2;
    example::foo f;  // This is example::v2::foo (if v1 was also inline, it would be ambiguous)
    f1.n1 = 1;
    f2.n2 = 2;
    f.n2 = 2;
}

f and f2 are the same type, but declared differently. Do we show the inline namespace for f2's tooltips and not f's? Do we never show the inline namespace for f2? Maybe nobody actually cares and we could go ahead and trim the inline namespace and wait to see if we get feedback asking for a setting to control the behavior.

[somedude72]

Having a setting in this case would be great because it would solve the case of "will developers expect to see the inline namespace for their types"; the developer could then adjust the behavior of hover tooltips to their expectations to always show, never show, or show the inline namespace if the source file has the qualifier.

That said, if creating a setting might be too much, I do think just trimming the inline namespace (or at least showing the inline namespace only if the qualifier is present in the definition of the symbol) is a viable solution. Because while it is somewhat more ambiguous as to where the symbol came from, trading a little bit of granularity for conciseness (while also being semantically correct) seems worthy.

There is also the simple and crude way of replacing std::__1 with just std when cpptools detects the clang header files.

[bobbrow]

There is also the simple and crude way of replacing std::__1 with just std when cpptools detects the clang header files.

Yes, that is also being discussed with the team.

[github-actions[bot]]

This feature request is being closed due to insufficient upvotes. Please leave a 👍-upvote or 👎-downvote reaction on the issue to help us prioritize it. When enough upvotes are received, this issue will be eligible for our backlog.