[PATCH 1/2] riched20: Stub for ITextFont interface and implement ITextRange::GetFont. (try 3)

[Jactry Zeng]

Hi Nikolay,

Thanks for your review!
I will have another try.

2014-09-23 23:01 GMT+08:00 Nikolay Sivov <bunglehead at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20140924/be06dfcd/attachment.html>