Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

For loop bugfix - explore enabling break and continue within for loops #216

Closed
lohvht-zz opened this issue Jun 13, 2018 · 11 comments
Closed
Assignees
Labels

Comments

@lohvht-zz
Copy link
Contributor

Attempt to fix the loss of continue and break for for-loops in TagUI scripts due to the IIFE pattern.

Proposed fix

upon translating from TagUI script to javascript, replace continues and breaks into returning corresponding flags.

then check the flag and carry out break/continue accordingly

@lohvht-zz lohvht-zz added the bug label Jun 13, 2018
@lohvht-zz lohvht-zz self-assigned this Jun 13, 2018
@kensoh
Copy link
Member

kensoh commented Jun 14, 2018

Made a commit in for_loop_break_continue branch - 8700c0c

samples/loop_demo can't work. in generated loop_demo.js, when for_loop_signal is set, the moment after // end of JS code line if put casper.echo(for_loop_signal); it shows blank.

casper.then(function() { // start of JS code
for_loop_signal = '[BREAK_SIGNAL]'; return;
}); // end of JS code

problem is variables set with casper.then() blocks remains in casper context. ie the moment exit IIFE pattern, for_loop_signal variable remains blanks and the code to break or continue does not trigger.

})(n); // end of IIFE pattern
if (for_loop_signal == '[BREAK_SIGNAL]') {for_loop_signal = ''; break;}
else if (for_loop_signal == '[CONTINUE_SIGNAL]') {for_loop_signal = ''; continue;}}
});

We can look into this together next week...

@kensoh
Copy link
Member

kensoh commented Jun 15, 2018

Experimental implementation using file IO doesn't work as well - 6cbf1f5

@kensoh kensoh changed the title For loop bugfix For loop bugfix - explore enabling break and continue within for loops Jun 15, 2018
@Aussiroth
Copy link
Contributor

kensoh added a commit that referenced this issue Jun 19, 2018
use casper.bypass() to count how many steps to teleport to and bypass when a break statement is issued. nice catch @Aussiroth !

Co-Authored-By: Alvin Yan <aussiroth@users.noreply.github.com>
Co-Authored-By: Victor Loh <lohvht@users.noreply.github.com>
@Aussiroth
Copy link
Contributor

Aussiroth commented Jun 19, 2018

TagUI script to test this issue here:

for n from 1 to 10
{
	echo n
	if n equals to 6
	{
		for k from 80 to 85
		{
			echo k
			if k equals to 83
			{
				break;
			}
		}
	}
}

for cde from 1 to 5
{
	echo "cde " + cde
}

for abc from 1 to 5
{
	echo "abc " + abc
}

Expected output:

1
2
3
4
5
6
81
82
83
7
8
9
10
cde 1
cde 2
cde 3
cde 4
cde 5
abc 1
abc 2
abc 3
abc 4
abc 5

Actual output:

1
2
3
4
5
6
7
81
82
83
abc 1
abc 2
abc 3
abc 4
abc 5

Issue with nested loop is likely due to finding the outermost for loop break signal and jumping to it, instead of the one for the inner for loop.
Unsure as to the weird behaviour for why the cde loop does not work, whereas the abc loop works whether cde loop is there or not.

kensoh added a commit that referenced this issue Jun 19, 2018
@kensoh
Copy link
Member

kensoh commented Jun 19, 2018

above commit gives following output. uses a teleport marker (variable of the loop) to track which loop to break. however, still not showing 7 8 9 10.

1
2
3
4
5
6
80
81
82
83
cde 1
cde 2
cde 3
cde 4
cde 5
abc 1
abc 2
abc 3
abc 4
abc 5

kensoh added a commit that referenced this issue Jun 19, 2018
@kensoh
Copy link
Member

kensoh commented Jun 19, 2018

above commit adds stricter conditions to give correct output. turns out the stack also contains the terminating marker, thus the outer loop for the n for loop will have the market for k for loop and results in false-positive match. below stricter condition to check for real match -

if (casper.steps[s].toString() == ("function () {for_loop_signal = '[BREAK_SIGNAL]["+teleport_marker+"]';}"))
1
2
3
4
5
6
80
81
82
83
7
8
9
10
cde 1
cde 2
cde 3
cde 4
cde 5
abc 1
abc 2
abc 3
abc 4
abc 5

@kensoh
Copy link
Member

kensoh commented Jun 19, 2018

looks ok for repeated use of for loop variables. would have thought will have issues as the markers are similar, but below works, haven't figure out why.

for n from 1 to 10
{       
        echo n
        if n equals to 6
        {       
                for k from 80 to 85
                {       
                        echo k
                        if k equals to 83
                        {       
                                break;
                        }
                }
        }
}

for cde from 1 to 5
{
        echo "cde " + cde
}

for abc from 1 to 5
{
        echo "abc " + abc
}

for n from 30 to 35
{
        echo n
}

for k from 20 to 25
{
        echo k
}
1
2
3
4
5
6
80
81
82
83
7
8
9
10
cde 1
cde 2
cde 3
cde 4
cde 5
abc 1
abc 2
abc 3
abc 4
abc 5
30
31
32
33
34
35
20
21
22
23
24
25

kensoh added a commit that referenced this issue Jun 19, 2018
@kensoh
Copy link
Member

kensoh commented Jun 20, 2018

Adding on, it works because the later steps in stack are not created yet, so later blocks of for loops with similar variable names wont trigger false-positive if a search is done from the back.

@kensoh
Copy link
Member

kensoh commented Jun 20, 2018

I'll clean up the code and add continue so that you guys can stress test... continue's implementation is probably adding a marker near the start of the loop and searching from forward direction instead of from the back.

reference debugging code that can be used in the last casper.then block in generated js

for (s = casper.steps.length-1; s >= 0; s--) {
casper.echo('\n\n\n\n\n' + s + ' ------------------------------------------------------------\n' + casper.steps[s].toString());}

kensoh added a commit that referenced this issue Jun 20, 2018
kensoh added a commit that referenced this issue Jun 20, 2018
doesn't work but don't break existing break test case.
@kensoh
Copy link
Member

kensoh commented Jun 20, 2018

Getting very close to final working version that works for continue or break step. for your stress / edge-cases testing..

to use, use continue or continue; or break or break; in a for loop

kensoh added a commit that referenced this issue Jun 20, 2018
eg

for n from 1 to infinity
{
echo n
}

if constant too big, takes very large to build stack before execution. trade-off decided to be 1024
kensoh added a commit that referenced this issue Jun 21, 2018
Co-Authored-By: Alvin Yan <aussiroth@users.noreply.github.com>
Co-Authored-By: Victor Loh <lohvht@users.noreply.github.com>

* #216 - experimenting with using signal variable
* #216 - possible solution found
use casper.bypass() to count how many steps to teleport to and bypass when a break statement is issued. nice catch @Aussiroth !
* #216 - use teleport marker
* #216 - use stricter matching conditions
* #216 - more complicated test
* #216 - cleaned up to work for break step
* #216 - initial implementation of continue
doesn't work but don't break existing break test case.
* #216 - working version of continue + break
* #216 - add infinity constant for looping
eg

for n from 1 to infinity
{
echo n
}

if constant too big, takes very large to build stack before execution. trade-off decided to be 1024
kensoh added a commit that referenced this issue Jun 21, 2018
Co-Authored-By: Alvin Yan <aussiroth@users.noreply.github.com>
Co-Authored-By: Victor Loh <lohvht@users.noreply.github.com>
@kensoh
Copy link
Member

kensoh commented Jun 21, 2018

merged to cutting edge release, now prepping to release v4.0

@kensoh kensoh closed this as completed Jun 21, 2018
kensoh added a commit to tebelorg/TagUI that referenced this issue Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants