Re: [PATCH 1/5] shell32: Check for ICommDlgBrowser from Site and use it in the IExplorerBrowser control.

[Nikolay Sivov]

On 8/26/2010 14:53, David Hedberg wrote:
> ---
>   dlls/shell32/ebrowser.c       |   98 +++++++++++++++++++----
>   dlls/shell32/tests/ebrowser.c |  174 ++++++++++++++++++++++++++++++++++++++++-
>   2 files changed, 251 insertions(+), 21 deletions(-)
>
> diff --git a/dlls/shell32/ebrowser.c b/dlls/shell32/ebrowser.c
> index b243aa0..1137880 100644
> --- a/dlls/shell32/ebrowser.c
> +++ b/dlls/shell32/ebrowser.c
> @@ -73,6 +73,8 @@ typedef struct _ExplorerBrowserImpl {
>       LPITEMIDLIST current_pidl;
>
> IUnknown *punk_site;
> +    ICommDlgBrowser3 *pcdb3_site;
> +    UINT cdb_site_supported;
>   } ExplorerBrowserImpl;
> I think a better way for this would be to store a pointer for each 
> interface version, instead of having additional switch. When you have 
> separate pointers a call looks more obvious like:

if (pcdb_site) ICommDlgBrowser_*(pcdb_site).

[PATCH 1/5] shell32: Check for ICommDlgBrowser from Site and use it in the IExplorerBrowser control.

[David Hedberg]

On Thu, Aug 26, 2010 at 13:15, Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
>  On 8/26/2010 14:53, David Hedberg wrote:
> > ---
> >  dlls/shell32/ebrowser.c       |   98 +++++++++++++++++++----
> >  dlls/shell32/tests/ebrowser.c |  174
> > ++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 251 insertions(+), 21 deletions(-)
> >
> > diff --git a/dlls/shell32/ebrowser.c b/dlls/shell32/ebrowser.c
> > index b243aa0..1137880 100644
> > --- a/dlls/shell32/ebrowser.c
> > +++ b/dlls/shell32/ebrowser.c
> > @@ -73,6 +73,8 @@ typedef struct _ExplorerBrowserImpl {
> >      LPITEMIDLIST current_pidl;
> >
> >      IUnknown *punk_site;
> > +    ICommDlgBrowser3 *pcdb3_site;
> > +    UINT cdb_site_supported;
> >  } ExplorerBrowserImpl;
>
> I think a better way for this would be to store a pointer for each interface
> version, instead of having additional switch. When you have separate
> pointers a call looks more obvious like:
>
> if (pcdb_site) ICommDlgBrowser_*(pcdb_site).

Makes sense, I'll send a new version. Thanks.