Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[doc-bug] Move Documentation Comments From .cpp To .hpp For Better LSP Support #828

Open
Saplyn opened this issue Mar 19, 2024 · 1 comment

Comments

@Saplyn
Copy link

Saplyn commented Mar 19, 2024

When installing TFXUI using CMake's find_package(), the implementation source code is not available, resulting LSP not being able to provide correct "hover document", as they're in the implementation files, according to this stackoverflow post comment.

For instance: that "--- Widget ---" cannot be the actual document:

unusful doc "--- Widget ---"

Here, the LSP (clangd) thinks the header file (elements.hpp) contains the documentation, but it does not:

// --- Widget ---
Element text(std::string text);

In fact, the actual documentation is written in the text.cpp:

/// @brief Display a piece of UTF8 encoded unicode text.
/// @ingroup dom
/// @see ftxui::to_wstring
///
/// ### Example
///
/// ```cpp
/// Element document = text("Hello world!");
/// ```
///
/// ### Output
///
/// ```bash
/// Hello world!
/// ```
Element text(std::string text) {
  return std::make_shared<Text>(std::move(text));
}

So... Maybe consider moving the actual documentation from implementation to declaration? As this brings better LSP support.

@ArthurSonzogni
Copy link
Owner

ArthurSonzogni commented May 1, 2024

I would like human to be able to quickly understand headers files. Doxygen comments adds a lot of noise making it more difficult to navigate the files. A lot of FTXUI documentations adds a lot of details, examples. This is too much.

  • FTXUI headers are 2792 lines.
  • FTXUI doxygen documentation is 2993 lines.

Is there a way to keep the headers easily navigeable by human, while allowing machine to provide the specific details?

About your issue, it seems to be a missing feature from clangd. We should probably push in this direction instead.
I see: llvm/llvm-project#67802

ArthurSonzogni added a commit that referenced this issue May 1, 2024
Fix all the diagnostics reported.

Bug: #828
ArthurSonzogni added a commit that referenced this issue May 1, 2024
Fix all the diagnostics reported.

Bug: #828
ArthurSonzogni added a commit that referenced this issue May 1, 2024
Fix all the diagnostics reported.

Bug: #828
ArthurSonzogni added a commit that referenced this issue May 1, 2024
Fix all the diagnostics reported.

Bug: #828
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants