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 MSVC debugger visualization file #5

Merged
merged 8 commits into from
Sep 21, 2018
Merged

Conversation

cmakshin
Copy link
Contributor

This pull request contains a file which can be used by the Visual Studio debugger for "pretty-printing" tsl::sparse_* containers.

@Tessil
Copy link
Owner

Tessil commented Sep 16, 2018

Hello,

I made a couple of corrections similar to the natvis of the ordered-map project.

I was also wondering if the visualization for tsl::sparse_map that shows the key in the name column was necessary? As it add extra code to maintain, what would be the advantage of having this view in addition to the first one?

@cmakshin
Copy link
Contributor Author

Hello.

Showing the key in the name column was done for two reasons:

  1. I find it more convenient for inspecting the container's contents;
  2. that's how std::map visualizer works [by default]. std::unordered_map visualizer also shows the key in the name column, but, for some reason, the whole key-value pair in the value column.

@Tessil
Copy link
Owner

Tessil commented Sep 17, 2018

Effectively.

I have to check a bit as the other natvis files you contributed just use a simple index (which is preferable for tsl::ordered_map). I have to see if I can add the key in the name of the column for the other projects in tsl as I would like to keep some consistency across the projects.

It doesn't make much sense to show the key in both columns and the
`std::map` visualizer shows only values in the value column. Key
duplication in `std::unordered_map` visualizer is probably a bug.
@Tessil
Copy link
Owner

Tessil commented Sep 20, 2018

I think it should be better to keep the (key, value) in the column index to mimic more closely what Microsoft did with std::unordered_map. If they change it in the future, I'll see to change it here too.

@Tessil Tessil merged commit aa6f281 into Tessil:master Sep 21, 2018
@Tessil
Copy link
Owner

Tessil commented Sep 21, 2018

I merged the changes, thank you for your contribution.

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