Skip to content

Using robots to fix/update docs part 3: com.h#654

Open
dunhor wants to merge 6 commits into
masterfrom
dunhor/com_docs
Open

Using robots to fix/update docs part 3: com.h#654
dunhor wants to merge 6 commits into
masterfrom
dunhor/com_docs

Conversation

@dunhor

@dunhor dunhor commented Jul 20, 2026

Copy link
Copy Markdown
Member

See #652 for more info on the effort

@dunhor
dunhor requested review from ChrisGuzak and jonwis July 20, 2026 19:16
@dunhor dunhor changed the title Using robots to fix/update docs part 2: com.h Using robots to fix/update docs part 3: com.h Jul 20, 2026
Comment thread include/wil/com.h Outdated

#ifdef WIL_ENABLE_EXCEPTIONS
//! COM pointer, errors throw exceptions (see @ref com_ptr_t for details)
//! COM pointer, errors throw exceptions (see @ref wil::com_ptr_t for details)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting; the docs system doesn't know about namespaces?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so apparently this is related to the change in #655 that pulled the end-of-wil-namespace curly brace outside of the @endcond comment. So, with that fix, the explicit namespace here is not necessary - Doxygen resolves the namespaces correctly.

And apparently, even without the explicit namespace here, Doxygen will make an educated guess based off which types are documented, so previously it was just a warning; the HTML was still the same and the links worked.

The net impact of this change is that the HTML now mentions wil::com_ptr_t instead of com_ptr_t. I guess it's personal preference which one you like more - since this is all documentation for WIL, the explicit wil namespace is a bit unnecessary.

I'll complete that PR first and then tidy this one up to remove the explicit namespaces and verify that the warnings are gone.

Comment thread include/wil/com.h Outdated
m_stream.reset();
}

//! Move constructor; transfers the saved stream position.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these comments necessary? Maybe put a better comment on the type itself, so what these methods do is implied?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, probably. I'll have it update the skill to wrap things like this w/ @cond/@endcond, though that means these constructors are no longer mentioned in the docs, which is probably fine.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, without comments, Doxygen will emit a warning, so there's effectively two options:

  1. Add docs to these declarations
  2. Wrap the declarations with begincond/endcond

I've changed to just wrapping with begincond/endcond. The "downside" is that these won't appear in the generated docs at all, even as undocumented. But given we can probably count the number of people using Doxygen with WIL on 0-1 hands, probably not a big deal to just omit them.

@dunhor

dunhor commented Jul 20, 2026

Copy link
Copy Markdown
Member Author

Interesting... Another NotifyPropertyChanged failure... seems I didn't completely fix it

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
witest.cpplatest.exe is a Catch2 v3.10.0 host application.
Run with -? for options

-------------------------------------------------------------------------------
CppWinRTAuthoringTests::NotifyPropertyChanged
-------------------------------------------------------------------------------
D:\a\wil\wil\tests\CppWinRTAuthoringTests.cpp(271)
...............................................................................

D:\a\wil\wil\tests\CppWinRTAuthoringTests.cpp(362): FAILED:
  REQUIRE( WaitForSingleObject(dispatcherThreadHandle.get(), 30 * 1000) == ((((DWORD )0x00000000L) ) + 0 ) )
with expansion:
  258 (0x102) == 0

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

Successfully merging this pull request may close these issues.

2 participants