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

Add Python script for generating Natvis file and update file for 3.11.2 #3697

Merged
merged 3 commits into from Aug 12, 2022

Conversation

falbrechtskirchinger
Copy link
Contributor

I updated GDB pretty printer for the new inline namespace but forgot Natvis. This PR adds a small Python script to generate the Natvis file from a Jinja2 template and generates the file for the 3.11.2 release. In the future, the release script should call ./tools/generate_natvis/generate_natvis.py --version X.Y.Z . (the trailing dot denoting the output directory).

Fixes #3696.

<Type Name="std::pair&lt;*, nlohmann::basic_json&lt;*&gt;&gt;" IncludeView="MapHelper">
<DisplayString>{second}</DisplayString>
<Expand>
<ExpandedItem>second</ExpandedItem>
</Expand>
</Type>

<!-- Namespace nlohmann::json_abi -->
<Type Name="nlohmann::json_abi::basic_json&lt;*&gt;">
Copy link
Contributor

@gregmarr gregmarr Aug 11, 2022

Choose a reason for hiding this comment

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

Is it possible to use wildcards for the namespace? nlohmann::*::basic_json...? If so, could we also hardcode the values for nlohmann::*::detail::value_t::*? This would allow a single natvis file that would work for any version?

Otherwise, would it make sense to include the version number in the filename so that users could have multiple version files installed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wildcards are only supported for template parameters according to the documentation.

Users will have to comment on whether versioning the Natvis file would be helpful. (ping @jheydebrand)

Copy link
Contributor

Choose a reason for hiding this comment

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

Wildcards are only supported for template parameters according to the documentation.

I saw documentation that said that they worked for templates, but it didn't specifically say that it was only used there, so I wasn't sure if it was limited to templates or just poorly documented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The fully qualified name of each Type element is specified in its Name attribute.

and

Templated classes

The Name attribute of the Type element accepts an asterisk * as a wildcard character that can be used for templated class names.

In the following example, the same visualization is used whether the object is a CAtlArray or a CAtlArray. If there's a specific visualization entry for a CAtlArray, then it takes precedence over the generic one.

The very specific wording suggests to me it only works for templates. 🤷‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

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

Confirmed that it only works for templates. :(

Choose a reason for hiding this comment

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

Thanks for the swift responses.

Versioning the natvis file would not provide any benefit for me.
When consuming the library via its CMake integration the correct / matching natvis file will be included anyway and I have no desire to copy the file to some system-wide directory (which is supported by Visual Studio and might have been the use-case for versioned natvis files you were thinking about?).

@falbrechtskirchinger
Copy link
Contributor Author

@nlohmann This should be included in 3.11.2 if possible. I could open a PR with just the updated Natvis file if you prefer.

@coveralls
Copy link

coveralls commented Aug 11, 2022

Coverage Status

Coverage remained the same at 100.0% when pulling 4b3a858 on falbrechtskirchinger:inline-ns-natvis into 0e61ee8 on nlohmann:develop.

Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@nlohmann nlohmann added this to the Release 3.11.2 milestone Aug 12, 2022
@nlohmann nlohmann merged commit c0dae0f into nlohmann:develop Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MSVC natvis visualizer does not work after introduction of inline ABI namespace
5 participants