2016-07-04 8:37 GMT-06:00 Alex Henrie alexhenrie24@gmail.com:
2016-07-04 5:09 GMT-06:00 Sebastian Lackner sebastian@fds-team.de:
On 04.07.2016 07:25, Alex Henrie wrote:
[...]
Some of the variable names (for example ppTInfo, rgszNames, rgDispId) sound a bit odd. Usually variable names without ugly prefix are preferred for new code (except we want to keep it for compatibility with old code here?).
I was trying to make the new code match the surrounding code. I don't think the variable names matter very much here, but I'll change them if you feel strongly about it.
+static HRESULT WINAPI FolderItemsImpl_Item(FolderItems3 *iface, VARIANT index, FolderItem **ppid) +{
- FIXME("(%p,%p)\n", iface, ppid);
You forgot to trace the index parameter here.
Good point. I'll fix that and resubmit the series (although hopefully Alexandre will have taken patch 1 without changes).
@@ -1093,8 +1297,7 @@ static HRESULT WINAPI FolderImpl_Items(Folder3 *iface, FolderItems **ppid) { FIXME("(%p,%p)\n", iface, ppid);
You forgot to change the FIXME to a TRACE.
This is actually intentional, because the patch makes this function return a list of items, but the list is empty. I'm going to change the FIXME to a TRACE in the patch that populates the list: https://github.com/alexhenrie/wine/commits/master
Thanks for the feedback.
-Alex
I grepped the code and the names ppTInfo, rgszNames, and rgDispId are used pretty consistently throughout Wine. I think that unless we're going to refactor the names across the entire codebase, we might as well keep them.
So, the only change I've made between v1 and v2 of this patchset is adding the missing parameter to FolderItemsImpl_Item's FIXME statement.
-Alex