aisingapore / TagUI

Free RPA tool by AI Singapore
Apache License 2.0
5.58k stars 580 forks source link

1. Improve synchronous execution of JS code and TagUI steps, 2. Support complex nested for loops with if, else if, else #150

Closed kensoh closed 6 years ago

kensoh commented 6 years ago

In some scenarios, the order of execution is not fully synchronous. The short summary is JavaScript is asynchronous by nature while CasperJS tries to make it synchronous -> but there is still gap in TagUI for some cases to convert human language into fully synchronous CasperJS JavaScript code.

Following shows an example and this issue has another example. In that previous issue, a potential fix was abandoned as it compromises backward compatibility for existing scripts. A big change in many parts of tagui_parse.php is needed to implement this fix.

There could be slight backward compatibility issues but this change is necessary to make automation scripts more deterministic and drive automation actions and behavior more consistently. Below the example is a list of items to evaluate and check before committing to master.

Replication example:

a=1
echo "checkpoint 1. a = " + a

if (a>0)
{
    a = 20
    echo "checkpoint 2. a = 20"    
}

echo "checkpoint 3. a = " + a

Output is not synchronous:

checkpoint 1. a = 1
checkpoint 3. a = 1
checkpoint 2. a = 20

List of items to evaluate on local bleeding edge version before committing to master -

Breaks in backward compatibility in exchange for deterministic automation execution

  1. var now strictly not supported for variables definition to be in scope. previously for some steps (eg echo, api), var variables will still be in scope
  2. frame and popup after one another requires { and } steps. in general conditions (for, if else, else if, while) and popup / frame need { and } steps if used consecutively to prevent ambiguity
  3. random use of { and } blocks without purpose no longer works. for example, when { and } steps are not used with if, else, else if, for, while popup, frame

Backward compatibility point 3 is now addressed by automatically supporting random code blocks by users that are not related to conditions or loops or frame or popup steps. I can't really think of a reason why a user would want to do that, perhaps for organizing sections of steps or keeping things readable. Nevertheless, updated codebase to support such random use of { and } to maximize backward compatbility.

For backward compatibility point 1, it is easy to update TagUI translation engine to always convert var variable_name = xxx to variable_name = xxx but this prevents advanced users who wants to limit scope of a variable within a section of JavaScript code from implementing good programming practices. Thus keep it flexible so that users can write variable_name = xxx according to TagUI documentation, or use var variable_name = xxx if they wish to limit scope for just that few consecutive JS statements.

For backward compatibility point 2, it is possible to write a pre-parsing algorithm in the translation engine to address that. However, that encourages bad scripting writing and ambiguity in code. Thus keeping it such that consecutive uses of conditions or loops or frame / popup steps require explicitly using { and } to define the code blocks in scope for such implementations. That promotes deterministic script-writing and easier maintenance as the script will show exactly what it is trying to do instead of leaving it to TagUI parsing engine to fill in the gaps.

kensoh commented 6 years ago

See list of items to evaluate and check on local bleeding edge version before committing to master.

kensoh commented 6 years ago

Above is an initial working commit for this, keeping issue open for any post commit issues.

More details of the issue, checklist and break in backward compatibility here - https://github.com/kelaberetiv/TagUI/issues/150#issue-319436500

Already cross-posted in #83. This feature is now usable from cutting edge version.

kensoh commented 6 years ago

Made above commit with following comments (to reduce backward compatibility)

After #150, random use of { and } blocks without purpose no longer works.

For example, when { and } steps are not used with if, else, else if, for, while popup, frame.

This commit addresses that, so that random use of { and } still works.
kensoh commented 6 years ago

Backward compatibility point 3 is now addressed by automatically supporting random code blocks by users that are not related to conditions or loops or frame or popup steps. I can't really think of a reason why a user would want to do that, perhaps for organizing sections of steps or keeping things readable. Nevertheless, updated codebase to support such random use of { and } to maximize backward compatbility.

For backward compatibility point 1, it is easy to update TagUI translation engine to always convert var variable_name = xxx to variable_name = xxx but this prevents advanced users who wants to limit scope of a variable within a section of JavaScript code from implementing good programming practices. Thus keep it flexible so that users can write variable_name = xxx according to TagUI documentation, or use var variable_name = xxx if they wish to limit scope for just that few consecutive JS statements.

For backward compatibility point 2, it is possible to write a pre-parsing algorithm in the translation engine to address that. However, that encourages bad scripting writing and ambiguity in code. Thus keeping it such that consecutive uses of conditions or loops or frame / popup steps require explicitly using { and } to define the code blocks in scope for such implementations. That promotes deterministic script-writing and easier maintenance as the script will show exactly what it is trying to do instead of leaving it to TagUI parsing engine to fill in the gaps.

This latest change is now usable from cutting edge version.

kensoh commented 6 years ago

User reported issue with below, which shows ERROR - number of step { does not tally with with }

js begin
if (a==1) a=20;
js finish

This has to be fixed to be backward compatible with existing scripts -> to look into a fix.

kensoh commented 6 years ago

Made commit above with following comments -

Code that are within integrations code blocks should skip special processing. This commit fixes that. For example, following should work and not having { and } auto-padded to after the if statement and after the js finish.

a = 1
js begin
if (a == 1) a = 2;
js finish
kensoh commented 6 years ago

Issue found for this overhaul with using non-english languages. Non-English languages such as Chinese, French, Russian, were never included in the positive test cases as that wasn't a critical part of TagUI features and use cases. This has to be resolved, tracking it here first on the issue.

kensoh commented 6 years ago

Above commit fixes translation issue for this overhaul, otherwise scripts using non-English languages won't work (eg Chinese, Serbian, Hungarian, German.. etc).

kensoh commented 6 years ago

Query from user on the var backward compatibility

Noticed that your new update does not support the below – we have been using the var functions a fair bit to move files around in the automation process, is there a new function that is replacing this?


My reply on the impact of that particular use scenario

It might work from what you are describing. Can you try, and see if your existing scripts using var breaks? var variable_name = 'xxx' defines a local variable which only exists in the context of that consecutive series of JavaScript statements. The lines of JS code immediately following the var line should still be able to access the variable just as before the overhaul.

In this overhaul, it will only not be accessible in some subsequent steps such as api step or echo step. Previously api or echo step would be able to access variables defined with var immediately preceding. As the break in backward compatibility is for a minor use scenario which was originally not intended, I opted to go ahead with the overhaul without automatically removing the var in user's JavaScript statements. If automatically remove var from the JavaScript lines, it will prevent advance users from explicitly declaring a local variable for use in that few lines of JS code.

kensoh commented 6 years ago

Closing issue for now since no further problems reported, this latest change is now usable from cutting edge version.

adegard commented 6 years ago

Hi @kensoh and thank you for this new release, I needed it for the same above problem and found it in your comment here.... Great!

How can I subscribe to be aware of each new releases? (is it some feed o email subscribtion is available?)

Thanks for your work ! I 'm using it intensivly and please let me know how can pay you (at least for a beer!)

kensoh commented 6 years ago

Hey @adegard it's our pleasure! I used to work on TagUI full-time on my own for a year, but now AI Singapore hires me to work on it and continue developing it. There isn't a mailing list now, would like to have incremental improvements every couple of weeks and releases every couple of months.

Thanks for your encouragement! I have an old paypal account. If you buy a drink and I'll probably use that to buy instant coffee to share with my friends in office. But there is really no need to pay, this is purely open-source contribution for people to have access to fun and useful automation tools!

kensoh commented 6 years ago

Thanks @adegard for your generosity!

Below is the outcome of your 'beer treat' ---> more coffee for conversion to code

slack

adegard commented 6 years ago

Your welcome Ken!

In two weeks with your help we made 3 bots for our little HR recluting agency:

We will continue to use it for the simplicity of language ("natural" vs pantomjs...) but also because it run headless (vs Uipath or other)

Have a nice day! Arnaud