<div dir="ltr">Hi Nikolay,<div><br></div><div>Thanks for your review!</div><div>I will have another try.</div></div><div class="gmail_extra"><br><div class="gmail_quote">2014-09-23 23:01 GMT+08:00 Nikolay Sivov <span dir="ltr"><<a href="mailto:bunglehead@gmail.com" target="_blank">bunglehead@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+typedef struct ITextFontImpl {<br>
+ ITextFont ITextFont_iface;<br>
+ LONG ref;<br>
+<br>
+ ITextRangeImpl *txtRge;<br>
+ ITextSelectionImpl *txtSel;<br>
+} ITextFontImpl;<br>
+<br>
</blockquote>
I think 'range' and 'selection' would be more readable, but what's more important you don't use 'txtSel' at all.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+static HRESULT WINAPI ITextFont_fnQueryInterface(<u></u>ITextFont *me, REFIID riid, void **ppvObj)<br>
</blockquote>
If we want some unification, it's usually called 'iface', not 'me'.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ *ppvObj = me;<br>
</blockquote>
That's not how it should look.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+static IRichEditOleImpl *get_reOle(ITextFontImpl *txtFont)<br>
+{<br>
+ if (txtFont->txtRge)<br>
+ return txtFont->txtRge->reOle;<br>
+ else<br>
+ return txtFont->txtSel->reOle;<br>
+}<br>
</blockquote>
Please use a better naming, like 'get_reole_from_font()' or something like that.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ FIXME("not implemented: %p\n", This);<br>
+ return E_NOTIMPL;<br>
</blockquote>
This should give a proper trace line with arguments, and it should appear on every call.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ ITextFontImpl *This = impl_from_ITextFont(me);<br>
+ if (pValue)<br>
+ return E_INVALIDARG;<br>
+ if (!get_reOle(This))<br>
+ return CO_E_RELEASED;<br>
+<br>
+ FIXME("Stub\n");<br>
+ *pValue = tomFalse;<br>
+ return S_OK;<br>
</blockquote>
That's another example of tracing done wrong.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
static HRESULT WINAPI ITextRange_fnGetFont(<u></u>ITextRange *me, ITextFont **pFont)<br>
{<br>
ITextRangeImpl *This = impl_from_ITextRange(me);<br>
+ ITextFontImpl *txtFont = NULL;<br>
+ HRESULT hres;<br>
+<br>
if (!This->reOle)<br>
return CO_E_RELEASED;<br>
- FIXME("not implemented %p\n", This);<br>
- return E_NOTIMPL;<br>
+ TRACE("%p\n", This);<br>
</blockquote>
And another.<br>
<br>
<br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div dir="ltr"><div>Regards,<br></div><div>Jactry Zeng</div><div><br></div></div>
</div>