Skip to content

Natvis cleanup following the windows-result crate split#2853

Merged
kennykerr merged 7 commits intomicrosoft:masterfrom
tim-weis:natvis
Feb 17, 2024
Merged

Natvis cleanup following the windows-result crate split#2853
kennykerr merged 7 commits intomicrosoft:masterfrom
tim-weis:natvis

Conversation

@tim-weis
Copy link
Contributor

Following up on PR 2847 this PR is tying up a few loose ends:

  • It splits windows-core's windows.natvis into windows-core.nativs and windows-result.natvis
  • Moves the individual .natvis files into their respective /natvis directories
  • Associates each .natvis file with its corresponding crate

This PR isolates debug visualizers into their respective crates, ensuring that the debugging experience is the same across any combination thereof.

This PR doesn't update the debugger_visualizer tests to validate that the visualizers work in isolation. As I understand, the testing process is destined to change, so I didn't bother. I'll happily rework the test code if that's still a ways off.

@kennykerr
Copy link
Collaborator

Thanks Tim, I'll leave #2854 open until this completes. I just need someone a bit more familiar with natvis to looks this over.

@tim-weis
Copy link
Contributor Author

Thank you, Kenny. I didn't mean to delay a release, and most certainly not an important one.

I'd feel more at ease if @ridwanabdillahi could have a look at the "changes" (or relocations). If that doesn't happen in due time feel free to release the crates (without this change) nonetheless. This PR doesn't affect existing clients, and is rather cosmetic in nature to future clients.

Copy link
Collaborator

@riverar riverar left a comment

Choose a reason for hiding this comment

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

The natvis split looks good to me.

Just noted another problem with our cross-debugger experience, something we'll have to start testing more thoroughly soon.

@kennykerr kennykerr merged commit a9b594b into microsoft:master Feb 17, 2024
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.

3 participants