[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wine-devel
Subject:    Re: riched20: ITextDocument stubs for ITextServices (retry)
From:       Jacek Caban <jacek () codeweavers ! com>
Date:       2013-07-30 9:00:50
Message-ID: 51F780C2.5070305 () codeweavers ! com
[Download RAW message or body]

On 07/29/13 17:55, Caibin Chen wrote:
> > +struct tagReTxtDoc {
> > +  IUnknown *outerObj;
> > +  ME_TextEditor *editor;
> > +  ITextDocument iTextDocumentIface;
> > +};
> > 
> > Do you really need a separated structure for that? Wouldn't adding ITextDocument \
> > implementation to ITextServicesImpl be enough?
> This structure will be used in both ITexstServicesImpl and
> IRichEditOleImpl(richole.c). For now IRichEditOleImpl has its own
> implementation(stub) of ITextDocument and ITextSelection. A stub of
> ITextSelection is needed before removing IRichEditOleImpl's own
> implementation and use the separate one.
> There are already several patches that are ready for review once this
> patch get submitted.

I see, assuming that substantial part of the implementation may really
be shared between those objects, that makes sense. Bellow is review of
the remaining part of the patch:

+struct tagReTxtDoc {
+  IUnknown *outerObj;
+  ME_TextEditor *editor;
+  ITextDocument iTextDocumentIface;
+};


Please follow http://wiki.winehq.org/COMGuideline about naming
convention (*_iface names).

+ITextDocument *ReTxtDoc_get_ITextDocument(ReTxtDoc *document)
+{
+  TRACE("(%p)\n", document);
+  return &document->iTextDocumentIface;
+}


Full declaration of ReTxtDoc in a header (so you don't need such helper) would be \
cleaner, IMO.


new file mode 100644
index 0000000..02579b2
--- /dev/null
+++ b/dlls/riched20/txtdoc.h


How about using an existing header?



Thanks,
Jacek


[prev in list] [next in list] [prev in thread] [next in thread]