2014-09-23 23:01 GMT+08:00 Nikolay Sivov <bunglehead@gmail.com>:

+typedef struct ITextFontImpl {
+    ITextFont ITextFont_iface;
+    LONG ref;
+
+    ITextRangeImpl *txtRge;
+    ITextSelectionImpl *txtSel;
+} ITextFontImpl;
+

I think 'range' and 'selection' would be more readable, but what's more important you don't use 'txtSel' at all.

+static HRESULT WINAPI ITextFont_fnQueryInterface(ITextFont *me, REFIID riid, void **ppvObj)

If we want some unification, it's usually called 'iface', not 'me'.

+        *ppvObj = me;

That's not how it should look.

+static IRichEditOleImpl *get_reOle(ITextFontImpl *txtFont)
+{
+  if (txtFont->txtRge)
+      return txtFont->txtRge->reOle;
+  else
+      return txtFont->txtSel->reOle;
+}

Please use a better naming, like 'get_reole_from_font()' or something like that.

+    FIXME("not implemented: %p\n", This);
+    return E_NOTIMPL;

This should give a proper trace line with arguments, and it should appear on every call.

+    ITextFontImpl *This = impl_from_ITextFont(me);
+    if (pValue)
+        return E_INVALIDARG;
+    if (!get_reOle(This))
+        return CO_E_RELEASED;
+
+    FIXME("Stub\n");
+    *pValue = tomFalse;
+    return S_OK;

That's another example of tracing done wrong.

  static HRESULT WINAPI ITextRange_fnGetFont(ITextRange *me, ITextFont **pFont)
  {
      ITextRangeImpl *This = impl_from_ITextRange(me);
+    ITextFontImpl *txtFont = NULL;
+    HRESULT hres;
+
      if (!This->reOle)
          return CO_E_RELEASED;
  -    FIXME("not implemented %p\n", This);
-    return E_NOTIMPL;
+    TRACE("%p\n", This);

And another.