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

Simpler package layout for natvis files #2862

Merged
merged 1 commit into from
Feb 21, 2024
Merged

Simpler package layout for natvis files #2862

merged 1 commit into from
Feb 21, 2024

Conversation

kennykerr
Copy link
Collaborator

I don't think we need the wordier file paths.

@kennykerr kennykerr merged commit 075c3c1 into master Feb 21, 2024
65 checks passed
@kennykerr kennykerr deleted the natvis branch February 21, 2024 02:09
@tim-weis
Copy link
Contributor

Seems reasonable.

I initially renamed the .natvis files expecting them to carry over into the PDBs and showing up in .nvlist. For some reason that didn't happen. (I'll sink some more time into investigating this when I can.)

The rationale behind moving them into a folder was to declutter the crate root. Doing that felt naive and I think leaving them in the crate root and marking them "hidden" (by convention) does a better job at that.

The only concern I have is whether this affects publishing. "Hidden" files (i.e., files starting with a .) follow special rules. Can you --dry-run a cargo publish to make sure they aren't getting lost? Not that the visualizers are stable enough to be of much help at this time, but things are moving.

@kennykerr
Copy link
Collaborator Author

I just published an update:

https://github.com/microsoft/windows-rs/releases/tag/0.53.0

Please do kick the tires and let me know if you find any issues. Thanks again for the help!

@tim-weis
Copy link
Contributor

Running .nvlist (WinDbg Preview) on a re-targeted binary displays both .natvis files (named <crate>-<index>.natvis), dumping the contents produces the expected files, and dx-ing the respective expressions delivers the desired results. (VS 2022 Preview still fails to expand the expressions.)

LGTM.

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