Last week of May

[Charles]

The last week of May will be rather light for me as I will be moving (cleaning, packing boxes, unpacking, etc.).

FYI

[eric.sellon]

Moving isn't very fun; hope it goes well for you. When you get a chance, I uploaded a patch file for Ticket 66 that you can bring in if you feel it's worthy

EDIT: Forgot to mention that when reporting the errors in the stmt method (of CobraParser.cobra), I used recordError instead of throwError (which the rest of the function was using). I did that so that the right location would be reported in the error message. Using throwError would report the error as being at the end of the previous line. I tested at least one of the other error messages in the function and it did have the same problem. So all the throwErrors in stmt should be changed to recordError (I didn't do that since it would've been outside the scope of the ticket; and you don't want me to willy nilly change things on you). My guess is that the problem is that stmt is peeking at the token and throwError is using .last to figure out which token was responsible for the error. Hope that helps.

[Charles]

Thanks for the patch. I'll take a look at it and a couple from hopscc after my move.

Re: .throwError, if you search for "def throwError" you'll find there are 2 of them: one that takes a token and one that invokes the other with .last. And then if you search for "def recordError" you'll find that it is the same. So the fix, for cases where .last does not work, is to simply pass the token in.

Use "throw" vs. "record" regarding whether the error can be recovered from successfully.

So they are two separate variables.

[eric.sellon]

Okay. So all the throwErrors in the stmt method should just get changed to using the form that accepts a token. Sounds good.

[Charles]

Moving isn't very fun; hope it goes well for you. When you get a chance, I uploaded a patch file for Ticket 66 that you can bring in if you feel it's worthy

I didn't see any test cases with the patch. We always need tests. Check out the ticket and let me know if you have any questions.

[eric.sellon]

I didn't see any test cases with the patch. We always need tests. Check out the ticket and let me know if you have any questions.

Patch file has been updated to address your concerns.

[Charles]

Thanks.

Applied with some tweaks (as I often do). Some more minor feedback:

-- "for x = 0 .. 2" is the old numeric for loop syntax. Some of these are still lurking in the compiler, but I don't want to go through and fix them until I'm more caught up on the patch backlog. The syntax is "for x in 2" or "for x in 1 : 5" for example. If you leave out the start, 0 is implied. The right end is excluded as with slicing (t[i:j]).

-- Re: boolean var names we use isFoo/hasFoo/didFoo/willFoo instead of fooFlag. Of course, use the style you prefer in your projects, but try to go with the flow in other projects.

-- We avoid print statements in most tests most of the time. I suppose for an error case it doesn't matter much, but I wanted to point it out in case you end up submitting more tests.

No criticisms here, just trying to get future patches to be even quicker to apply.