[PATCH 1/2] [PATCH 1/2] jscript: (partly) implement Enumerator object

Jacek Caban jacek at codeweavers.com
Wed Apr 17 11:51:48 CDT 2019


Hi Andreas,

Thanks for the patch.

On 4/15/19 9:26 PM, Andreas Maier wrote:
> Enumerator is an MS extension.
> This fixes installatiion of VyChat219.msi.
>
> Signed-off-by: Andreas Maier<staubim at quantentunnel.de>
> ---
>   dlls/jscript/Makefile.in   |   1 +
>   dlls/jscript/enumerator.c  | 455 +++++++++++++++++++++++++++++++++++++++++++++
>   dlls/jscript/global.c      |  18 +-
>   dlls/jscript/jscript.h     |  10 +
>   dlls/jscript/jsglobal.idl  |   3 -
>   dlls/jscript/tests/rsrc.rc |   3 +
>   dlls/jscript/tests/run.c   |   1 +
>   7 files changed, 480 insertions(+), 11 deletions(-)
>   create mode 100644 dlls/jscript/enumerator.c


Please split the patch. Eg. send a stub implementation first that has 
only a constructor that creates an instance with its methods returning 
E_NOTIMPL first, something that would allow a one-line test to pass:

new Enumerator([]);

That part could mostly go as is, I have some comments to object 
implementations bellow.


> +#define DATATYPE_ARRAY           0
> +#define DATATYPE_STRINGLIST      1
> +#define DATATYPE_DRIVECOLLECTION 2
> +
> +typedef struct {
> +    jsdisp_t dispex;
> +
> +    int index;
> +    int length;
> +    int datatype;
> +    /* constructor with jsarray e.g. ["A","B"] */
> +    jsdisp_t *array;
> +    /* constructor with stringlist-interface
> +       methods Items and Count are available.
> +       e.g. StringList or IDriveCollection-Interface */
> +    IDispatch *stringlist;
> +} EnumeratorInstance;


It doesn't look right to have to implement it for each particular 
interface. We will likely need separate code path for internal jscript 
objects and general IDipsatch objects, but we shouldn't need to know 
anything about IDriveCollection here. This will need more testing. I've 
attached a tiny first step for such investigation. It uses Enumerator 
constructor on test-provided object. If you run it on Windows, you will 
see that it tries to call Invoke(DISPID_NEWENUM) and fails because we 
don't implement that. My guess would be that it needs DISPID_NEWENUM to 
succeed and then queries for IEnumVARIANT on the result, but it would 
need more testing. If my guess is right, we'd need just IEnumVARIANT 
pointer here.


> diff --git a/dlls/jscript/jsglobal.idl b/dlls/jscript/jsglobal.idl
> index b8604e99f2..7e0c1a30cf 100644
> --- a/dlls/jscript/jsglobal.idl
> +++ b/dlls/jscript/jsglobal.idl
> @@ -74,9 +74,6 @@ library JSGlobal
>           [id(DISPID_GLOBAL_VBARRAY)]
>           VARIANT VBArray();
>
> -        [id(DISPID_GLOBAL_ENUMERATOR)]
> -        VARIANT Enumerator();
> -
>           [id(DISPID_GLOBAL_ESCAPE)]
>           VARIANT escape(VARIANT String);


Why do you remove it?


> diff --git a/dlls/jscript/tests/rsrc.rc b/dlls/jscript/tests/rsrc.rc
> index d9ee6b23ae..b130df294a 100644
> --- a/dlls/jscript/tests/rsrc.rc
> +++ b/dlls/jscript/tests/rsrc.rc
> @@ -25,6 +25,9 @@ cc.js 40 "cc.js"
>   /* @makedep: lang.js */
>   lang.js 40 "lang.js"
>
> +/* @makedep: extension.js */
> +extension.js 40 "extension.js"
> +
>   /* @makedep: regexp.js */
>   regexp.js 40 "regexp.js"
>
> diff --git a/dlls/jscript/tests/run.c b/dlls/jscript/tests/run.c
> index aadb31700d..9e92a418f1 100644
> --- a/dlls/jscript/tests/run.c
> +++ b/dlls/jscript/tests/run.c
> @@ -2684,6 +2684,7 @@ static BOOL run_tests(void)
>
>       run_from_res("lang.js");
>       run_from_res("api.js");
> +    run_from_res("extension.js");


Please put tests into existing files. For plain JavaScript code, api.js 
would be appropriate. For test checking interaction with host objects, 
having short scripts inside run.c will be probably better.


Thanks,

Jacek

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20190417/e9f31cb0/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jscript.diff
Type: text/x-patch
Size: 924 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20190417/e9f31cb0/attachment-0001.bin>


More information about the wine-devel mailing list