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