Hi Nikolay,

Thanks for your review!
I will have another try.

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.





--
Regards,
Jactry Zeng