Autocomplete using ComWrappers#6685
Conversation
kant2002
force-pushed
the
kant/autocomplete
branch
from
5e96979 to
90d5f66
Compare
| /// </summary> | ||
| public StringSource(string[] strings) | ||
| { | ||
| Array.Clear(strings, 0, size); |
There was a problem hiding this comment.
Based on the usage this will never ever happens.
There was a problem hiding this comment.
Or better say, this is no-op.
There was a problem hiding this comment.
While it is true that there are no callers that pass in a null array, not clearing the passed in array is a change in behavior. I wouldn't remove the clear in this change.
|
|
||
| public void RefreshList(string[] newSource) | ||
| { | ||
| Array.Clear(strings, 0, size); |
There was a problem hiding this comment.
Also based on the usage this will never happens.
There was a problem hiding this comment.
Or better say, this is no-op.
|
|
||
| strings = newSource; | ||
| current = 0; | ||
| size = (strings is null) ? 0 : strings.Length; |
There was a problem hiding this comment.
Line 78 - yes. But Line 80 definitely can be simplified, since strings never null.
|
Does this require additional review? |
Yes. @JeremyKuhne may want to look and merge. give it a day or two more. |
| { | ||
| private IntPtr _wrappedInstance; | ||
|
|
||
| public AutoCompleteWrapper(IntPtr wrappedInstance) |
There was a problem hiding this comment.
IAutoComplete2 has methods Init, Enable, SetOptions, and GetOptions. Here for AutoCompleteWrapper I only see Init and SetOptions. Is that intentional?
There was a problem hiding this comment.
Yes. This is intentional. There no need for declaring methods in RCW if they would not be used in the application. WinForms use just 2 methods, so I leave only these 2 method. That mentality was shown for me in the dotnet/runtime repo, and based on my experience working with WinForms team, same applied here as well. Additional cost (albeit very small) is paid for all WinForms consumers.
There was a problem hiding this comment.
That is good information to know for ComWrappers, I will keep this in mind. Thank you for the insight.
ghost
locked as resolved and limited conversation to collaborators
5 participants
Provide RCW for
Provides CCW
Microsoft Reviewers: Open in CodeFlow