Re: [PATCH 1/4] vbscript: Implemented TypeName(try 2)

[Jacek Caban]

On 06/17/14 07:58, Shuai Meng wrote:
> iff --git a/dlls/vbscript/global.c b/dlls/vbscript/global.c
> index 5160374..5357df8 100644
> --- a/dlls/vbscript/global.c
> +++ b/dlls/vbscript/global.c
> @@ -1502,8 +1502,57 @@ static HRESULT Global_DatePart(vbdisp_t *This, VARIANT *arg, unsigned args_cnt,
>
> static HRESULT Global_TypeName(vbdisp_t *This, VARIANT *arg, unsigned args_cnt, VARIANT *res)
>  {
> - FIXME("\n");
> - return E_NOTIMPL;
> - static const WCHAR ByteW[]     = {'B', 'y', 't', 'e', 0};
> - static const WCHAR IntegerW[]  = {'I', 'n', 't', 'e', 'g', 'e', 'r', 0};
> - static const WCHAR LongW[]     = {'L', 'o', 'n', 'g', 0};
> - static const WCHAR SingleW[]   = {'S', 'i', 'n', 'g', 'l', 'e', 0};
> - static const WCHAR DoubleW[]   = {'D', 'o', 'u', 'b', 'l', 'e', 0};
> - static const WCHAR CurrencyW[] = {'C', 'u', 'r', 'r', 'e', 'n', 'c', 'y', 0};
> - static const WCHAR DecimalW[]  = {'D', 'e', 'c', 'i', 'm', 'a', 'l', 0};
> - static const WCHAR DateW[]     = {'D', 'a', 't', 'e', 0};
> - static const WCHAR StringW[]   = {'S', 't', 'r', 'i', 'n', 'g', 0};
> - static const WCHAR BooleanW[]  = {'B', 'o', 'o', 'l', 'e', 'a', 'n', 0};
> - static const WCHAR EmptyW[]    = {'E', 'm', 'p', 't', 'y', 0};
> - static const WCHAR NullW[]     = {'N', 'u', 'l', 'l', 0};
> - /*static const WCHAR ObjectW[]   = {'O', 'b', 'j', 'e', 'c', 't', 0};

Please don't leave such commented things in the patch. You could remove
it, but ideally this should have tests and implementation.
> - static const WCHAR NothingW[]  = {'N', 'o', 't', 'h', 'i', 'n', 'g', 0};*/
> -
> - TRACE("(%s)\n", debugstr_variant(arg));
> -
> - assert(args_cnt == 1);
> -
> - if(res) {

This if() is not needed. return_string will take care of NULL res.
> -
>    switch(V_VT(arg)) {
> -
>            case VT_UI1:

Please don't add 8 space indention. I personally prefer no additional
indentions for cases, but in any case, if you do indention, use 4 spaces
as the rest of file.
> -
>                return return_string(res, ByteW);
> -
>            case VT_I2:
> -
>                return return_string(res, IntegerW);
> -
>            case VT_I4:
> -
>                return return_string(res, LongW);
> -
>            case VT_R4:
> -
>                return return_string(res, SingleW);
> -
>            case VT_R8:
> -
>                return return_string(res, DoubleW);
> -
>            case VT_CY:
> -
>                return return_string(res, CurrencyW);
> -
>            case VT_DECIMAL:
> -
>                return return_string(res, DecimalW);
> -
>            case VT_DATE:
> -
>                return return_string(res, DateW);
> -
>            case VT_BSTR:
> -
>                return return_string(res, StringW);
> -
>            case VT_BOOL:
> -
>                return return_string(res, BooleanW);
> -
>            case VT_EMPTY:
> -
>                return return_string(res, EmptyW);
> -
>            case VT_NULL:
> -
>                return return_string(res, NullW);
> -
>            default:
> -
>                FIXME("arg %s not supported\n", debugstr_variant(arg));
> -
>                return E_NOTIMPL;
> -
>    }
> - }
> - return S_OK;
>
> }
>
> static HRESULT Global_Array(vbdisp_t *This, VARIANT *arg, unsigned args_cnt, VARIANT *res)
> diff --git a/dlls/vbscript/tests/api.vbs b/dlls/vbscript/tests/api.vbs
> index 2cf9bb7..32f4dc3 100644
> --- a/dlls/vbscript/tests/api.vbs
> +++ b/dlls/vbscript/tests/api.vbs
> @@ -845,4 +845,25 @@ MyObject.myval = 0
>  Call ok(CSng(MyObject) = 0, "CSng(MyObject) = " & CSng(MyObject))
>  Call ok(getVT(CSng(MyObject)) = "VT_R4", "getVT(CSng(MyObject)) = " & getVT(CSng(MyObject)))
>
> +Call ok(TypeName(CByte(255)) = "Byte", "TypeName(CByte(255)) = " & TypeName(CByte(255)))
> +Call ok(getVT(TypeName(CByte(255))) = "VT_BSTR", "getVT(TypeName(CByte(255))) = " & getVT(TypeName(CByte(255))))
> +Call ok(TypeName(255) = "Integer", "TypeName(255) = " & TypeName(255))
> +Call ok(getVT(TypeName(255)) = "VT_BSTR", "getVT(TypeName(255)) = " & getVT(TypeName(255)))
> +Call ok(TypeName(32768) = "Long", "TypeName(32768) = " & TypeName(32768))
> +Call ok(getVT(TypeName(32768)) = "VT_BSTR", "getVT(TypeName(32768)) = " & getVT(TypeName(32768)))
> +Call ok(TypeName(CSng(0.5)) = "Single", "TypeName(CSng(0.5)) = " & TypeName(CSng(0.5)))
> +Call ok(getVT(TypeName(CSng(0.5))) = "VT_BSTR", "getVT(TypeName(CSng(0.5))) = " & getVT(TypeName(CSng(0.5))))
> +Call ok(TypeName(-0.5) = "Double", "TypeName(-0.5) = " & TypeName(-0.5))
> +Call ok(getVT(TypeName(-0.5)) = "VT_BSTR", "getVT(TypeName(-0.5)) = " & getVT(TypeName(-0.5)))
> +Call ok(TypeName(CCur(0.5)) = "Currency", "TypeName(CCur(0.5)) = " & TypeName(CCur(0.5)))
> +Call ok(getVT(TypeName(CCur(0.5))) = "VT_BSTR", "getVT(TypeName(CCur(0.5))) = " & getVT(TypeName(CCur(0.5))))
> +Call ok(TypeName(CStr(0.5)) = "String", "TypeName(CStr(0.5)) = " & TypeName(CStr(0.5)))
> +Call ok(getVT(TypeName(CStr(0.5))) = "VT_BSTR", "getVT(TypeName(CStr(0.5))) = " & getVT(TypeName(CStr(0.5))))
> +Call ok(TypeName(CBool(0.5)) = "Boolean", "TypeName(CBool(0.5)) = " & TypeName(CBool(0.5)))
> +Call ok(getVT(TypeName(CBool(0.5))) = "VT_BSTR", "getVT(TypeName(CBool(0.5))) = " & getVT(TypeName(CBool(0.5))))

Using True or False directly instead of CBool() would be cleaner.

Jacek