Add PD and PD_RESULT enums in Interop Comdlg32#2594
Merged
RussKie merged 1 commit intodotnet:masterfrom Jan 2, 2020
Merged
Conversation
gpetrou
force-pushed
the
PD
branch
3 times, most recently
from
8a7e4df to
cf484bf
Compare
Codecov Report
@@ Coverage Diff @@
## master #2594 +/- ##
==================================================
+ Coverage 57.02795% 57.04975% +0.0218%
==================================================
Files 1440 1440
Lines 417476 417476
Branches 39004 39004
==================================================
+ Hits 238078 238169 +91
+ Misses 174011 173933 -78
+ Partials 5387 5374 -13
|
hughbe
suggested changes
Dec 30, 2019
ghost
added
waiting-author-feedback
RussKie
suggested changes
Jan 2, 2020
ghost
added
the
waiting-author-feedback
RussKie
added
the
enhancement
ghost
removed
the
waiting-author-feedback
RussKie
reviewed
Jan 2, 2020
RussKie
previously approved these changes
Jan 2, 2020
RussKie
approved these changes
Jan 2, 2020
Contributor
|
@gpetrou have you tested the functionality to confirm the print dialog was shown? |
Contributor
Author
|
@RussKie sorry about that. If I remember correctly, I had only tested the changes before modifying the flags to use Comdlg32.PD. I shouldn't have changed the order of class items. |
weltkante
reviewed
Feb 13, 2020
| IntPtr m_hDevNames; | ||
| IntPtr m_hDC; | ||
|
|
||
| int m_Flags; |
Contributor
There was a problem hiding this comment.
fields cannot be moved around
| public IntPtr hDevNames { get { return m_hDevNames; } set { m_hDevNames = value; } } | ||
| public IntPtr hDC { get { return m_hDC; } set { m_hDC = value; } } | ||
|
|
||
| public int Flags { get { return m_Flags; } set { m_Flags = value; } } |
Contributor
There was a problem hiding this comment.
automatic properties must be avoided in interop since you don't control the binary layout anymore
[edit] made the comment on the wrong side of the diff, this line got replaced by an automatic property public int Flags { get; set; } which is what I was talking about above.
| IntPtr m_hDevNames; | ||
| IntPtr m_hDC; | ||
|
|
||
| int m_Flags; |
Contributor
There was a problem hiding this comment.
fields cannot be moved around
Contributor
There was a problem hiding this comment.
Ah, should have picked that! 🤦♂
| public IntPtr hDevNames { get { return m_hDevNames; } set { m_hDevNames = value; } } | ||
| public IntPtr hDC { get { return m_hDC; } set { m_hDC = value; } } | ||
|
|
||
| public int Flags { get { return m_Flags; } set { m_Flags = value; } } |
Contributor
There was a problem hiding this comment.
don't use automatic properties since you cannot control the binary layout
[edit] made the comment on the wrong side of the diff, this line got replaced by an automatic property public int Flags { get; set; } which is what I was talking about above.
RussKie
added a commit
to RussKie/winforms
that referenced
this pull request
Feb 28, 2020
Resolves dotnet#2814 The bug was caused by the conversion of a struct fields to auto-properties in dotnet#2594. This broke the interop binary layout. Rework the code to - correctly define x86 and x64 structs - make the structs blittable - add tests verifying the structs layouts
M-Lipin
pushed a commit
to M-Lipin/winforms
that referenced
this pull request
Mar 23, 2020
Resolves dotnet#2814 The bug was caused by the conversion of a struct fields to auto-properties in dotnet#2594. This broke the interop binary layout. Rework the code to - correctly define x86 and x64 structs - make the structs blittable - add tests verifying the structs layouts
ghost
locked as resolved and limited conversation to collaborators
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Microsoft Reviewers: Open in CodeFlow