Fix handling of for loop limits in boundary cases.

[bwkimmel]

• The step parameter needs to default to a non-zero value when start == limit, Otherwise, the following does not print anything (but should print "1"):

  for [i 1 1] [ print :i ]  ; This *should* print "1".

• The step parameter also needs to be evaluated before entering the loop. Otherwise, it takes on the value 0 during the first test. This causes the following two lines to print nothing (they should both print "1"):

  for [i 1 1 1] [ print :i ]  ; This *should* print "1".
  for [i 1 1 -1] [ print :i ]  ; This *should* print "1".

  And also causes the following to run through the loop once, printing the start value (they should print nothing):

  for [i 1 2 -1] [ print :i ]  ; This *should* print nothing.
  for [i 2 1 1] [ print :i ]  ; This *should* print nothing.

[inexorabletash]

Thanks for the patch! Can you add tests?

On Sep 1, 2017, at 9:33 PM, Brad Kimmel notifications@github.com wrote:

The step parameter needs to default to a non-zero value when start == limit, Otherwise, the following does not print anything (but should print "1"):

for [i 1 1] [ print :i ] ; This should print "1". The step parameter also needs to be evaluated before entering the loop. Otherwise, it takes on the value 0 during the first test. This causes the following two lines to print nothing (they should both print "1"):

for [i 1 1 1] [ print :i ] ; This should print "1". for [i 1 1 -1] [ print :i ] ; This should print "1". And also causes the following to run through the loop once, printing the start value (they should print nothing):

for [i 1 2 -1] [ print :i ] ; This should print nothing. for [i 2 1 1] [ print :i ] ; This should print nothing. You can view, comment on, or merge this pull request online at:

https://github.com/inexorabletash/jslogo/pull/104

Commit Summary

Fix handling of for loop limits in boundary cases. File Changes

M logo.js (10) Patch Links:

https://github.com/inexorabletash/jslogo/pull/104.patch https://github.com/inexorabletash/jslogo/pull/104.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[bwkimmel]

Done.

One consequence of this change is that the step parameter is only evaluated once from outside the scope of the for loop (i.e., the same way start and stop are evaluated). So one of the tests (which re-evaluates step each time from within the scope of the loop -- and using the loop variable as the step size) no longer passes.

I verified, though, that this new behavior is what UCB logo does.