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

Remove LIST_VIEW_INSERT_MARK_FLAGS enum #1701

Merged
merged 2 commits into from
Sep 11, 2023

Conversation

elachlan
Copy link
Contributor

@elachlan elachlan commented Sep 9, 2023

*** Comparing .winmd to last release
Unknown differences found:
Windows.Win32.UI.Controls.Apis.LVIM_AFTER added
Windows.Win32.UI.Controls.LIST_VIEW_INSERT_MARK_FLAGS removed
Windows.Win32.UI.Controls.LIST_VIEW_INSERT_MARK_FLAGS.LVIM_AFTER removed
Windows.Win32.UI.Controls.LVINSERTMARK.dwFlags...Windows.Win32.UI.Controls.LIST_VIEW_INSERT_MARK_FLAGS => System.UInt32

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.

Where did you get this value from? Not seeing it in public headers.

@elachlan
Copy link
Contributor Author

elachlan commented Sep 9, 2023

The insertion point appears after the item specified if the LVIM_AFTER flag is set; otherwise it appears before the specified item.

https://learn.microsoft.com/en-us/windows/win32/api/commctrl/ns-commctrl-lvinsertmark

It doesn't explicitly set it in the headers but the documentation says if its not AFTER then its before.

@elachlan
Copy link
Contributor Author

The other thing to consider is that LIST_VIEW_INSERT_MARK_FLAGS enum before this only had one value.

@riverar
Copy link
Collaborator

riverar commented Sep 11, 2023

The only thing I can find in headers is:

#define LVIM_AFTER      0x00000001

While I agree that when the flag is not set a logical before occurs, I don't like the idea of metadata making up constants. (Also consider that values 2, 4, 8, etc. will also leave bit 0 unset.)

Will defer to @mikebattista for the final call though!

@elachlan
Copy link
Contributor Author

If we end up deciding not to add it, then I suggest we remove the enum and use the constants.

This is how we currently define it in Winforms:
https://github.com/dotnet/winforms/blob/478c4e1d28e9adf29938c135aedbb31be47289fd/src/System.Windows.Forms.Primitives/src/Interop/ComCtl32/Interop.LVIM.cs

@mikebattista
Copy link
Contributor

If we end up deciding not to add it, then I suggest we remove the enum and use the constants.

This is how we currently define it in Winforms: https://github.com/dotnet/winforms/blob/478c4e1d28e9adf29938c135aedbb31be47289fd/src/System.Windows.Forms.Primitives/src/Interop/ComCtl32/Interop.LVIM.cs

I would rather remove the enum than create a new artificial enum member.

@elachlan elachlan changed the title Add LVIM_BEFORE to LIST_VIEW_INSERT_MARK_FLAGS to better represent enum choices Remove LIST_VIEW_INSERT_MARK_FLAGS enum Sep 11, 2023
@elachlan
Copy link
Contributor Author

PR is updated to remove the enum.

@mikebattista mikebattista merged commit 84b1491 into microsoft:main Sep 11, 2023
1 check passed
@mikebattista
Copy link
Contributor

Thanks!

@elachlan elachlan deleted the LIST_VIEW_INSERT_MARK_FLAGS branch September 11, 2023 20:58
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