Re: [PATCH v2 2/8] dhtmled.ocx: Add stub DLL and stub DHTMLEdit object

[Jacek Caban]

Hi Alex,

On 06.12.2017 06:32, Alex Henrie wrote:
> 2017-12-05 6:45 GMT-07:00 Jacek Caban :
> > On 05.12.2017 10:09, Alex Henrie wrote:
> > Signed-off-by: Alex Henrie
> >
> > configure.ac                         |   1 +
> >  dlls/dhtmled.ocx/Makefile.in         |   8 +
> >  dlls/dhtmled.ocx/dhtmled.ocx.spec    |   4 +
> >  dlls/dhtmled.ocx/dhtmled_classes.idl |  39 +++
> >  dlls/dhtmled.ocx/dhtmled_private.h   |  19 ++
> >  dlls/dhtmled.ocx/dhtmled_tlb.idl     |  21 ++
> >  dlls/dhtmled.ocx/edit.c              | 534
> > +++++++++++++++++++++++++++++++++++
> >  dlls/dhtmled.ocx/main.c              | 162 +++++++++++
> >  8 files changed, 788 insertions(+)
> >  create mode 100644 dlls/dhtmled.ocx/Makefile.in
> >  create mode 100644 dlls/dhtmled.ocx/dhtmled.ocx.spec
> >  create mode 100644 dlls/dhtmled.ocx/dhtmled_classes.idl
> >  create mode 100644 dlls/dhtmled.ocx/dhtmled_private.h
> >  create mode 100644 dlls/dhtmled.ocx/dhtmled_tlb.idl
> >  create mode 100644 dlls/dhtmled.ocx/edit.c
> >  create mode 100644 dlls/dhtmled.ocx/main.c
> >
> > It would be nice to split it a bit. For example separated patches for stub
> > dll, adding .idl file and adding DHTMLEdit stub.
>
> I don't think it makes sense to stub the DLL, then add an IDL, then
> add the object implementation, because the DLL is worthless without
> the IDL and the IDL is worthless without the object implementation. I
> have taken care to only add the most basic features, and even that is
> already divided across 8 patches. Nonetheless, if you feel strongly
> that this patch should be split into three, I'll do it.

No, in this case I don't feel strongly about it. It just makes the whole
process easier (for example, I could have already signed off stub dll
part yesterday if it was a separated patch) and is trivial to do with
git. I also don't really agree it's about your justifications about
separated things being worthless. A new DLL is always a functional
change visible to applications. Also a new IDL adds and registers a new
typelib, which is useful even without corresponding implementation.

Anyway, in this case, one patch is fine with me.
> > diff --git a/dlls/dhtmled.ocx/dhtmled_classes.idl
> > b/dlls/dhtmled.ocx/dhtmled_classes.idl
> > new file mode 100644
> > index 0000000000..ae27c7c14a
> > --- /dev/null
> > +++ b/dlls/dhtmled.ocx/dhtmled_classes.idl
> > @@ -0,0 +1,39 @@
> > +/*
> > -
> > - Copyright 2017 Alex Henrie
> > -
> > -
> > -
> > - This library is free software; you can redistribute it and/or
> > -
> > - modify it under the terms of the GNU Lesser General Public
> > -
> > - License as published by the Free Software Foundation; either
> > -
> > - version 2.1 of the License, or (at your option) any later version.
> > -
> > -
> > -
> > - This library is distributed in the hope that it will be useful,
> > -
> > - but WITHOUT ANY WARRANTY; without even the implied warranty of
> > -
> > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > -
> > - Lesser General Public License for more details.
> > -
> > -
> > -
> > - You should have received a copy of the GNU Lesser General Public
> > -
> > - License along with this library; if not, write to the Free Software
> > -
> > - Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301,
> >
> > USA
> > - */
> > -
> >
> > +#pragma makedep register
> >
> > I think we also want typelib.
>
> dhtmled_tlb.idl already has "#pragma makedep regtypelib". Are you
> saying that it needs "#pragma makedep typelib" too?

That's fine, I missed that.
> > -
> >
> > +[
> > - uuid(2d360201-fff5-11d1-8d03-00a0c959bc0a),
> > - threading(apartment)
> >
> > +]
> > +coclass DHTMLSafe
> > +{
> > - interface IDHTMLSafe;
> > - interface _DHTMLSafeEvents;
> >
> > +};
> > -
> >
> > +[
> > - uuid(2d360200-fff5-11d1-8d03-00a0c959bc0a),
> > - threading(apartment)
> >
> > +]
> > +coclass DHTMLEdit
> > +{
> > - interface IDHTMLEdit;
> > - interface _DHTMLEditEvents;
> >
> > +};
> >
> > Ideally it would just #include dhtmled.idl (see mshtml.tlb for an example).
>
> I did that for dhtmled_classes.idl. Can I combine dhtmled_classes.idl
> and dhtmled_tlb.idl into one file with both pragmas?

Yes, that would be better.

Thanks,
Jacek